Repository: badges/shields (js)

Issue:
Perform query param validation within BaseService
We've a few badges which validate query params by invoking `_validateQueryParams` on BaseService.

If we provided the route with a Joi schema instead of an array of names, we could validate these _within_ BaseService instead – i.e. prior to invoking `handle()`.

If #2675 hasn't been completed yet, we could use a placeholder `Joi.object().required()` for those services in the meantime.

Pull request:
Validate query params in BaseService [npm nodeping mozillaobservatory matrix gitlab f-droid endpoint dynamic bitbucket appveyor]
This is a mid-sized PR that adds query param validation to BaseService and updates most of the services which use query param validation to use it. There are a couple minor tweaks I made along the way.

Fix #2676

Patch:
diff --git a/core/base-service/base-svg-scraping.spec.js b/core/base-service/base-svg-scraping.spec.js
index 2cc8af5c2f..15548e0ba6 100644
--- a/core/base-service/base-svg-scraping.spec.js
+++ b/core/base-service/base-svg-scraping.spec.js
@@ -124,6 +124,10 @@ describe('BaseSvgScrapingService', function() {
 
     it('allows overriding the valueMatcher', async function() {
       class WithValueMatcher extends BaseSvgScrapingService {
+        static get route() {
+          return {}
+        }
+
         async handle() {
           return this._requestSvg({
             schema,
diff --git a/core/base-service/base-xml.spec.js b/core/base-service/base-xml.spec.js
index 05ac79b59c..aeba56ee69 100644
--- a/core/base-service/base-xml.spec.js
+++ b/core/base-service/base-xml.spec.js
@@ -56,6 +56,10 @@ describe('BaseXmlService', function() {
 
     it('forwards options to _sendAndCacheRequest', async function() {
       class WithCustomOptions extends BaseXmlService {
+        static get route() {
+          return {}
+        }
+
         async handle() {
           const { requiredString } = await this._requestXml({
             schema: dummySchema,
diff --git a/core/base-service/base.js b/core/base-service/base.js
index b08b1a3bcb..5398a1caaf 100644
--- a/core/base-service/base.js
+++ b/core/base-service/base.js
@@ -18,6 +18,7 @@ const {
   assertValidRoute,
   prepareRoute,
   namedParamsForMatch,
+  getQueryParamNames,
 } = require('./route')
 const { assertValidServiceDefinition } = require('./service-definitions')
 const trace = require('./trace')
@@ -174,7 +175,8 @@ module.exports = class BaseService {
 
     let base, format, pattern, queryParams
     try {
-      ;({ base, format, pattern, query: queryParams = [] } = this.route)
+      ;({ base, format, pattern } = this.route)
+      queryParams = getQueryParamNames(this.route)
     } catch (e) {
       // Legacy services do not have a route.
     }
@@ -270,12 +272,44 @@ module.exports = class BaseService {
 
     const serviceInstance = new this(context, config)
 
+    let serviceError
+    const { queryParamSchema } = this.route
+    if (queryParamSchema) {
+      try {
+        queryParams = validate(
+          {
+            ErrorClass: InvalidParameter,
+            prettyErrorMessage: 'invalid query parameter',
+            includeKeys: true,
+            traceErrorMessage: 'Query params did not match schema',
+            traceSuccessMessage: 'Query params after validation',
+          },
+          queryParams,
+          queryParamSchema
+        )
+        trace.logTrace(
+          'inbound',
+          emojic.crayon,
+          'Query params after validation',
+          queryParams
+        )
+      } catch (error) {
+        serviceError = error
+      }
+    }
+
     let serviceData
-    try {
-      serviceData = await serviceInstance.handle(namedParams, queryParams)
-      Joi.assert(serviceData, serviceDataSchema)
-    } catch (error) {
-      serviceData = serviceInstance._handleError(error)
+    if (!serviceError) {
+      try {
+        serviceData = await serviceInstance.handle(namedParams, queryParams)
+        Joi.assert(serviceData, serviceDataSchema)
+      } catch (error) {
+        serviceError = error
+      }
+    }
+
+    if (serviceError) {
+      serviceData = serviceInstance._handleError(serviceError)
     }
 
     trace.logTrace('outbound', emojic.shield, 'Service data', serviceData)
@@ -286,11 +320,12 @@ module.exports = class BaseService {
   static register({ camp, handleRequest, githubApiProvider }, serviceConfig) {
     const { cacheHeaders: cacheHeaderConfig, fetchLimitBytes } = serviceConfig
     const { regex, captureNames } = prepareRoute(this.route)
+    const queryParams = getQueryParamNames(this.route)
 
     camp.route(
       regex,
       handleRequest(cacheHeaderConfig, {
-        queryParams: this.route.queryParams,
+        queryParams,
         handler: async (queryParams, match, sendBadge, request) => {
           const namedParams = namedParamsForMatch(captureNames, match, this)
           const serviceData = await this.invoke(
@@ -343,20 +378,6 @@ module.exports = class BaseService {
     )
   }
 
-  static _validateQueryParams(queryParams, queryParamSchema) {
-    return validate(
-      {
-        ErrorClass: InvalidParameter,
-        prettyErrorMessage: 'invalid query parameter',
-        includeKeys: true,
-        traceErrorMessage: 'Query params did not match schema',
-        traceSuccessMessage: 'Query params after validation',
-      },
-      queryParams,
-      queryParamSchema
-    )
-  }
-
   async _request({ url, options = {}, errorMessages = {} }) {
     const logTrace = (...args) => trace.logTrace('fetch', ...args)
     logTrace(emojic.bowAndArrow, 'Request', url, '\n', options)
diff --git a/core/base-service/base.spec.js b/core/base-service/base.spec.js
index f4b27b7ad0..e002f26c14 100644
--- a/core/base-service/base.spec.js
+++ b/core/base-service/base.spec.js
@@ -16,6 +16,15 @@ const BaseService = require('./base')
 
 require('../register-chai-plugins.spec')
 
+const queryParamSchema = Joi.object({
+  queryParamA: Joi.string(),
+})
+  .rename('legacyQueryParamA', 'queryParamA', {
+    ignoreUndefined: true,
+    override: true,
+  })
+  .required()
+
 class DummyService extends BaseService {
   static render({ namedParamA, queryParamA }) {
     return {
@@ -50,7 +59,7 @@ class DummyService extends BaseService {
     return {
       base: 'foo',
       pattern: ':namedParamA',
-      queryParams: ['queryParamA'],
+      queryParamSchema,
     }
   }
 }
@@ -71,6 +80,21 @@ describe('BaseService', function() {
     })
   })
 
+  it('Validates query params', async function() {
+    expect(
+      await DummyService.invoke(
+        {},
+        defaultConfig,
+        { namedParamA: 'bar.bar.bar' },
+        { queryParamA: ['foo', 'bar'] }
+      )
+    ).to.deep.equal({
+      color: 'red',
+      isError: true,
+      message: 'invalid query parameter: queryParamA',
+    })
+  })
+
   describe('Required overrides', function() {
     it('Should throw if render() is not overridden', function() {
       expect(() => BaseService.render()).to.throw(
@@ -78,12 +102,26 @@ describe('BaseService', function() {
       )
     })
 
-    it('Should throw if handle() is not overridden', async function() {
+    it('Should throw if route is not overridden', async function() {
       try {
         await BaseService.invoke({}, {}, {})
         expect.fail('Expected to throw')
       } catch (e) {
-        expect(e.message).to.equal('Handler not implemented for BaseService')
+        expect(e.message).to.equal('Route not defined for BaseService')
+      }
+    })
+
+    class WithRoute extends BaseService {
+      static get route() {
+        return {}
+      }
+    }
+    it('Should throw if handle() is not overridden', async function() {
+      try {
+        await WithRoute.invoke({}, {}, {})
+        expect.fail('Expected to throw')
+      } catch (e) {
+        expect(e.message).to.equal('Handler not implemented for WithRoute')
       }
     })
 
@@ -127,7 +165,7 @@ describe('BaseService', function() {
       expect(trace.logTrace).to.be.calledWith(
         'inbound',
         sinon.match.string,
-        'Query params',
+        'Query params after validation',
         { queryParamA: '!' }
       )
     })
@@ -281,7 +319,15 @@ describe('BaseService', function() {
 
     it('handles the request', async function() {
       expect(mockHandleRequest).to.have.been.calledOnce
-      const { handler: requestHandler } = mockHandleRequest.getCall(0).args[1]
+
+      const {
+        queryParams: serviceQueryParams,
+        handler: requestHandler,
+      } = mockHandleRequest.getCall(0).args[1]
+      expect(serviceQueryParams).to.deep.equal([
+        'queryParamA',
+        'legacyQueryParamA',
+      ])
 
       const mockSendBadge = sinon.spy()
       const mockRequest = {
@@ -328,7 +374,7 @@ describe('BaseService', function() {
         isDeprecated: false,
         route: {
           pattern: '/foo/:namedParamA',
-          queryParams: [],
+          queryParams: ['queryParamA', 'legacyQueryParamA'],
         },
       })
       // The in-depth tests for examples reside in transform-example.spec.js
@@ -352,18 +398,6 @@ describe('BaseService', function() {
         expect(e).to.be.an.instanceof(InvalidResponse)
       }
     })
-
-    it('throws error for invalid query params', async function() {
-      try {
-        DummyService._validateQueryParams(
-          { requiredString: ['this', "shouldn't", 'work'] },
-          dummySchema
-        )
-        expect.fail('Expected to throw')
-      } catch (e) {
-        expect(e).to.be.an.instanceof(InvalidParameter)
-      }
-    })
   })
 
   describe('request', function() {
diff --git a/core/base-service/route.js b/core/base-service/route.js
index c154db4cdb..711d7d5ca4 100644
--- a/core/base-service/route.js
+++ b/core/base-service/route.js
@@ -15,9 +15,11 @@ const routeSchema = Joi.object({
     is: Joi.string().required(),
     then: Joi.array().items(Joi.string().required()),
   }),
+  queryParamSchema: Joi.object().schema(),
   queryParams: Joi.array().items(Joi.string().required()),
 })
   .xor('pattern', 'format')
+  .oxor('queryParamSchema', 'queryParams')
   .required()
 
 function assertValidRoute(route, message = undefined) {
@@ -67,9 +69,19 @@ function namedParamsForMatch(captureNames = [], match, ServiceClass) {
   return result
 }
 
+function getQueryParamNames({ queryParams = [], queryParamSchema }) {
+  if (queryParamSchema) {
+    const { children, renames = [] } = Joi.describe(queryParamSchema)
+    return Object.keys(children).concat(renames.map(({ from }) => from))
+  } else {
+    return queryParams
+  }
+}
+
 module.exports = {
   makeFullUrl,
   assertValidRoute,
   prepareRoute,
   namedParamsForMatch,
+  getQueryParamNames,
 }
diff --git a/core/base-service/route.spec.js b/core/base-service/route.spec.js
index 9dadb0a196..86607976cf 100644
--- a/core/base-service/route.spec.js
+++ b/core/base-service/route.spec.js
@@ -1,8 +1,13 @@
 'use strict'
 
 const { expect } = require('chai')
+const Joi = require('joi')
 const { test, given, forCases } = require('sazerac')
-const { prepareRoute, namedParamsForMatch } = require('./route')
+const {
+  prepareRoute,
+  namedParamsForMatch,
+  getQueryParamNames,
+} = require('./route')
 
 describe('Route helpers', function() {
   context('A `pattern` with a named param is declared', function() {
@@ -101,4 +106,20 @@ describe('Route helpers', function() {
       'Service MyService declares incorrect number of named params (expected 2, got 1)'
     )
   })
+
+  it('getQueryParamNames', function() {
+    expect(getQueryParamNames({ queryParams: ['foo'] })).to.deep.equal(['foo'])
+    expect(
+      getQueryParamNames({
+        queryParamSchema: Joi.object({ foo: Joi.string() }).required(),
+      })
+    ).to.deep.equal(['foo'])
+    expect(
+      getQueryParamNames({
+        queryParamSchema: Joi.object({ foo: Joi.string() })
+          .rename('bar', 'foo', { ignoreUndefined: true, override: true })
+          .required(),
+      })
+    ).to.deep.equal(['foo', 'bar'])
+  })
 })
diff --git a/services/bitbucket/bitbucket-pull-request.service.js b/services/bitbucket/bitbucket-pull-request.service.js
index 496983e5bf..f7561ca85a 100644
--- a/services/bitbucket/bitbucket-pull-request.service.js
+++ b/services/bitbucket/bitbucket-pull-request.service.js
@@ -3,13 +3,17 @@
 const Joi = require('joi')
 const serverSecrets = require('../../lib/server-secrets')
 const { metric } = require('../../lib/text-formatters')
-const { nonNegativeInteger } = require('../validators')
+const { nonNegativeInteger, optionalUrl } = require('../validators')
 const { BaseJsonService } = require('..')
 
 const bitbucketPullRequestSchema = Joi.object({
   size: nonNegativeInteger,
 }).required()
 
+const queryParamSchema = Joi.object({
+  server: optionalUrl,
+}).required()
+
 function pullRequestClassGenerator(raw) {
   const routePrefix = raw ? 'pr-raw' : 'pr'
   const badgeSuffix = raw ? '' : ' open'
@@ -98,7 +102,7 @@ function pullRequestClassGenerator(raw) {
       return {
         base: `bitbucket/${routePrefix}`,
         pattern: `:user/:repo`,
-        queryParams: ['server'],
+        queryParamSchema,
       }
     }
 
diff --git a/services/dynamic/dynamic-helpers.js b/services/dynamic/dynamic-helpers.js
index e0ab46a2b3..6921857674 100644
--- a/services/dynamic/dynamic-helpers.js
+++ b/services/dynamic/dynamic-helpers.js
@@ -3,14 +3,6 @@
 const Joi = require('joi')
 const { optionalUrl } = require('../validators')
 
-function createRoute(which) {
-  return {
-    base: `badge/dynamic/${which}`,
-    pattern: '',
-    queryParams: ['uri', 'url', 'query', 'prefix', 'suffix'],
-  }
-}
-
 const queryParamSchema = Joi.object({
   url: optionalUrl.required(),
   query: Joi.string().required(),
@@ -20,7 +12,14 @@ const queryParamSchema = Joi.object({
   .rename('uri', 'url', { ignoreUndefined: true, override: true })
   .required()
 
+function createRoute(which) {
+  return {
+    base: `badge/dynamic/${which}`,
+    pattern: '',
+    queryParamSchema,
+  }
+}
+
 module.exports = {
   createRoute,
-  queryParamSchema,
 }
diff --git a/services/dynamic/dynamic-json.service.js b/services/dynamic/dynamic-json.service.js
index 55fa9819ec..abce5fc774 100644
--- a/services/dynamic/dynamic-json.service.js
+++ b/services/dynamic/dynamic-json.service.js
@@ -4,7 +4,7 @@ const Joi = require('joi')
 const jp = require('jsonpath')
 const { BaseJsonService, InvalidResponse } = require('..')
 const { renderDynamicBadge, errorMessages } = require('../dynamic-common')
-const { createRoute, queryParamSchema } = require('./dynamic-helpers')
+const { createRoute } = require('./dynamic-helpers')
 
 module.exports = class DynamicJson extends BaseJsonService {
   static get category() {
@@ -21,14 +21,7 @@ module.exports = class DynamicJson extends BaseJsonService {
     }
   }
 
-  async handle(namedParams, queryParams) {
-    const {
-      url,
-      query: pathExpression,
-      prefix,
-      suffix,
-    } = this.constructor._validateQueryParams(queryParams, queryParamSchema)
-
+  async handle(namedParams, { url, query: pathExpression, prefix, suffix }) {
     const data = await this._requestJson({
       schema: Joi.any(),
       url,
diff --git a/services/dynamic/dynamic-xml.service.js b/services/dynamic/dynamic-xml.service.js
index 1bb2edc65f..189532e774 100644
--- a/services/dynamic/dynamic-xml.service.js
+++ b/services/dynamic/dynamic-xml.service.js
@@ -4,7 +4,7 @@ const { DOMParser } = require('xmldom')
 const xpath = require('xpath')
 const { BaseService, InvalidResponse } = require('..')
 const { renderDynamicBadge, errorMessages } = require('../dynamic-common')
-const { createRoute, queryParamSchema } = require('./dynamic-helpers')
+const { createRoute } = require('./dynamic-helpers')
 
 // This service extends BaseService because it uses a different XML parser
 // than BaseXmlService which can be used with xpath.
@@ -27,14 +27,7 @@ module.exports = class DynamicXml extends BaseService {
     }
   }
 
-  async handle(namedParams, queryParams) {
-    const {
-      url,
-      query: pathExpression,
-      prefix,
-      suffix,
-    } = this.constructor._validateQueryParams(queryParams, queryParamSchema)
-
+  async handle(namedParams, { url, query: pathExpression, prefix, suffix }) {
     const pathIsAttr = pathExpression.includes('@')
 
     const { buffer } = await this._request({
diff --git a/services/dynamic/dynamic-yaml.service.js b/services/dynamic/dynamic-yaml.service.js
index 6ccc56c4ed..800467f29a 100644
--- a/services/dynamic/dynamic-yaml.service.js
+++ b/services/dynamic/dynamic-yaml.service.js
@@ -4,7 +4,7 @@ const Joi = require('joi')
 const jp = require('jsonpath')
 const { BaseYamlService, InvalidResponse } = require('..')
 const { renderDynamicBadge, errorMessages } = require('../dynamic-common')
-const { createRoute, queryParamSchema } = require('./dynamic-helpers')
+const { createRoute } = require('./dynamic-helpers')
 
 module.exports = class DynamicYaml extends BaseYamlService {
   static get category() {
@@ -21,14 +21,7 @@ module.exports = class DynamicYaml extends BaseYamlService {
     }
   }
 
-  async handle(namedParams, queryParams) {
-    const {
-      url,
-      query: pathExpression,
-      prefix,
-      suffix,
-    } = this.constructor._validateQueryParams(queryParams, queryParamSchema)
-
+  async handle(namedParams, { url, query: pathExpression, prefix, suffix }) {
     const data = await this._requestYaml({
       schema: Joi.any(),
       url,
diff --git a/services/endpoint/endpoint.service.js b/services/endpoint/endpoint.service.js
index 083bc7ef65..79ff34ff11 100644
--- a/services/endpoint/endpoint.service.js
+++ b/services/endpoint/endpoint.service.js
@@ -22,7 +22,7 @@ module.exports = class Endpoint extends BaseJsonService {
     return {
       base: 'badge/endpoint',
       pattern: '',
-      queryParams: ['url'],
+      queryParamSchema,
     }
   }
 
@@ -66,12 +66,7 @@ module.exports = class Endpoint extends BaseJsonService {
     }
   }
 
-  async handle(namedParams, queryParams) {
-    const { url } = this.constructor._validateQueryParams(
-      queryParams,
-      queryParamSchema
-    )
-
+  async handle(namedParams, { url }) {
     const { protocol, hostname } = new URL(url)
     if (protocol !== 'https:') {
       throw new InvalidParameter({ prettyMessage: 'please use https' })
diff --git a/services/f-droid/f-droid.service.js b/services/f-droid/f-droid.service.js
index 046bf92df9..3f2b6279c0 100644
--- a/services/f-droid/f-droid.service.js
+++ b/services/f-droid/f-droid.service.js
@@ -11,6 +11,10 @@ const schema = Joi.object({
     .required(),
 }).required()
 
+const queryParamSchema = Joi.object({
+  metadata_format: Joi.string().valid(['yml', 'txt']),
+}).required()
+
 module.exports = class FDroid extends BaseYamlService {
   static render({ version }) {
     return {
@@ -19,9 +23,7 @@ module.exports = class FDroid extends BaseYamlService {
     }
   }
 
-  async handle({ appId }, queryParams) {
-    const constructor = this.constructor
-    const { metadata_format: format } = constructor.validateParams(queryParams)
+  async handle({ appId }, { metadata_format: metadataFormat }) {
     const url = `https://gitlab.com/fdroid/fdroiddata/raw/master/metadata/${appId}`
     const fetchOpts = {
       options: {},
@@ -29,7 +31,7 @@ module.exports = class FDroid extends BaseYamlService {
         404: 'app not found',
       },
     }
-    const fetch = format === 'yml' ? this.fetchYaml : this.fetchText
+    const fetch = metadataFormat === 'yml' ? this.fetchYaml : this.fetchText
     let result
 
     try {
@@ -38,14 +40,14 @@ module.exports = class FDroid extends BaseYamlService {
       // on f-droid, so if txt is not found we look for yml as the fallback
       result = await fetch.call(this, url, fetchOpts)
     } catch (error) {
-      if (format) {
+      if (metadataFormat) {
         // if the format was specified it doesn't make the fallback request
         throw error
       }
       result = await this.fetchYaml(url, fetchOpts)
     }
 
-    return constructor.render(result)
+    return this.constructor.render(result)
   }
 
   async fetchYaml(url, options) {
@@ -82,14 +84,6 @@ module.exports = class FDroid extends BaseYamlService {
     return { version: match[1] }
   }
 
-  static validateParams(queryParams) {
-    const queryParamsSchema = Joi.object({
-      metadata_format: Joi.string().valid(['yml', 'txt']),
-    }).required()
-
-    return this._validateQueryParams(queryParams, queryParamsSchema)
-  }
-
   // Metadata
   static get defaultBadgeData() {
     return { label: 'f-droid' }
@@ -103,7 +97,7 @@ module.exports = class FDroid extends BaseYamlService {
     return {
       base: 'f-droid/v',
       pattern: ':appId',
-      queryParams: ['metadata_format'],
+      queryParamSchema,
     }
   }
 
diff --git a/services/gitlab/gitlab-pipeline-status.service.js b/services/gitlab/gitlab-pipeline-status.service.js
index b1686e75cc..91dedb22c0 100644
--- a/services/gitlab/gitlab-pipeline-status.service.js
+++ b/services/gitlab/gitlab-pipeline-status.service.js
@@ -27,7 +27,7 @@ module.exports = class GitlabPipelineStatus extends BaseSvgScrapingService {
     return {
       base: 'gitlab/pipeline',
       pattern: ':user/:repo/:branch*',
-      queryParams: ['gitlab_url'],
+      queryParamSchema,
     }
   }
 
@@ -63,10 +63,10 @@ module.exports = class GitlabPipelineStatus extends BaseSvgScrapingService {
     return renderBuildStatusBadge({ status })
   }
 
-  async handle({ user, repo, branch = 'master' }, queryParams) {
-    const {
-      gitlab_url: baseUrl = 'https://gitlab.com',
-    } = this.constructor._validateQueryParams(queryParams, queryParamSchema)
+  async handle(
+    { user, repo, branch = 'master' },
+    { gitlab_url: baseUrl = 'https://gitlab.com' }
+  ) {
     const { message: status } = await this._requestSvg({
       schema: badgeSchema,
       url: `${baseUrl}/${user}/${repo}/badges/${branch}/pipeline.svg`,
diff --git a/services/matrix/matrix.service.js b/services/matrix/matrix.service.js
index 17b10bf75c..8e910710a7 100644
--- a/services/matrix/matrix.service.js
+++ b/services/matrix/matrix.service.js
@@ -170,11 +170,7 @@ module.exports = class Matrix extends BaseJsonService {
     }
   }
 
-  async handle({ roomAlias }, queryParams) {
-    const { server_fqdn: serverFQDN } = this.constructor._validateQueryParams(
-      queryParams,
-      queryParamSchema
-    )
+  async handle({ roomAlias }, { server_fqdn: serverFQDN }) {
     const members = await this.fetch({ roomAlias, serverFQDN })
     return this.constructor.render({ members })
   }
@@ -191,7 +187,7 @@ module.exports = class Matrix extends BaseJsonService {
     return {
       base: 'matrix',
       pattern: ':roomAlias',
-      queryParams: ['server_fqdn'],
+      queryParamSchema,
     }
   }
 
diff --git a/services/mozilla-observatory/mozilla-observatory.service.js b/services/mozilla-observatory/mozilla-observatory.service.js
index 54da067470..b54b963014 100644
--- a/services/mozilla-observatory/mozilla-observatory.service.js
+++ b/services/mozilla-observatory/mozilla-observatory.service.js
@@ -1,8 +1,8 @@
 'use strict'
 
+const Joi = require('joi')
 const { BaseJsonService } = require('..')
 
-const Joi = require('joi')
 const schema = Joi.object({
   state: Joi.string()
     .valid('ABORTED', 'FAILED', 'FINISHED', 'PENDING', 'STARTING', 'RUNNING')
@@ -26,6 +26,10 @@ const schema = Joi.object({
     .required(),
 }).required()
 
+const queryParamSchema = Joi.object({
+  publish: Joi.equal(''),
+}).required()
+
 const documentation = `
 <p>
   The <a href="https://observatory.mozilla.org">Mozilla HTTP Observatory</a>
@@ -35,7 +39,7 @@ const documentation = `
 </p>
   By default the scan result is hidden from the public result list.
   You can activate the publication of the scan result
-  by setting <code>publish</code> parameter to <code>true</code>
+  by setting the <code>publish</code> parameter.
 <p>
 <p>
   The badge returns a cached site result if the site has been scanned anytime in the previous 24 hours.
@@ -55,7 +59,7 @@ module.exports = class MozillaObservatory extends BaseJsonService {
     return {
       base: 'mozilla-observatory',
       pattern: ':which(grade|grade-score)/:host',
-      queryParams: ['publish'],
+      queryParamSchema,
     }
   }
 
diff --git a/services/nodeping/nodeping-status.service.js b/services/nodeping/nodeping-status.service.js
index 987e3fb6dc..6c55ee3c7a 100644
--- a/services/nodeping/nodeping-status.service.js
+++ b/services/nodeping/nodeping-status.service.js
@@ -3,18 +3,23 @@
 const { BaseJsonService } = require('..')
 const Joi = require('joi')
 
-const rowSchema = Joi.object().keys({ su: Joi.boolean() })
-
 const schema = Joi.array()
-  .items(rowSchema)
+  .items(Joi.object().keys({ su: Joi.boolean() }))
   .min(1)
 
+const queryParamSchema = Joi.object({
+  up_message: Joi.string(),
+  down_message: Joi.string(),
+  up_color: Joi.alternatives(Joi.string(), Joi.number()),
+  down_color: Joi.alternatives(Joi.string(), Joi.number()),
+}).required()
+
 /*
  * this is the checkUuid for the NodePing.com (as used on the [example page](https://nodeping.com/reporting.html#results))
  */
-const sampleCheckUuid = 'jkiwn052-ntpp-4lbb-8d45-ihew6d9ucoei'
+const exampleCheckUuid = 'jkiwn052-ntpp-4lbb-8d45-ihew6d9ucoei'
 
-class NodePingStatus extends BaseJsonService {
+module.exports = class NodePingStatus extends BaseJsonService {
   static get category() {
     return 'monitoring'
   }
@@ -29,7 +34,7 @@ class NodePingStatus extends BaseJsonService {
     return {
       base: 'nodeping/status',
       pattern: ':checkUuid',
-      queryParams: ['up_message', 'down_message', 'up_color', 'down_color'],
+      queryParamSchema,
     }
   }
 
@@ -38,24 +43,24 @@ class NodePingStatus extends BaseJsonService {
       {
         title: 'NodePing status',
         namedParams: {
-          checkUuid: sampleCheckUuid,
+          checkUuid: exampleCheckUuid,
         },
         staticPreview: this.render({ status: true }),
       },
       {
         title: 'NodePing status (customized)',
         namedParams: {
-          checkUuid: sampleCheckUuid,
+          checkUuid: exampleCheckUuid,
         },
         queryParams: {
-          up_message: 'Online',
+          up_message: 'online',
           up_color: 'blue',
-          down_message: 'Offline',
+          down_message: 'offline',
           down_color: 'lightgrey',
         },
         staticPreview: this.render({
           status: true,
-          upMessage: 'Online',
+          upMessage: 'online',
           upColor: 'blue',
         }),
       },
@@ -101,5 +106,3 @@ class NodePingStatus extends BaseJsonService {
       : { message: downMessage || 'down', color: downColor || 'red' }
   }
 }
-
-module.exports = NodePingStatus
diff --git a/services/npm/npm-base.js b/services/npm/npm-base.js
index a6553a143f..326455f465 100644
--- a/services/npm/npm-base.js
+++ b/services/npm/npm-base.js
@@ -3,6 +3,7 @@
 const Joi = require('joi')
 const serverSecrets = require('../../lib/server-secrets')
 const { BaseJsonService, InvalidResponse, NotFound } = require('..')
+const { optionalUrl } = require('../validators')
 const { isDependencyMap } = require('../package-json-helpers')
 
 const deprecatedLicenseObjectSchema = Joi.object({
@@ -30,6 +31,10 @@ const packageDataSchema = Joi.object({
     .default([]),
 }).required()
 
+const queryParamSchema = Joi.object({
+  registry_uri: optionalUrl,
+}).required()
+
 // Abstract class for NPM badges which display data about the latest version
 // of a package.
 module.exports = class NpmBase extends BaseJsonService {
@@ -38,13 +43,13 @@ module.exports = class NpmBase extends BaseJsonService {
       return {
         base,
         pattern: ':scope(@[^/]+)?/:packageName/:tag?',
-        queryParams: ['registry_uri'],
+        queryParamSchema,
       }
     } else {
       return {
         base,
         pattern: ':scope(@[^/]+)?/:packageName',
-        queryParams: ['registry_uri'],
+        queryParamSchema,
       }
     }
   }


Test patch:
diff --git a/services/appveyor/appveyor-tests.service.js b/services/appveyor/appveyor-tests.service.js
index af16b60608..2b7f5fe740 100644
--- a/services/appveyor/appveyor-tests.service.js
+++ b/services/appveyor/appveyor-tests.service.js
@@ -1,5 +1,6 @@
 'use strict'
 
+const Joi = require('joi')
 const { renderTestResultBadge } = require('../../lib/text-formatters')
 const AppVeyorBase = require('./appveyor-base')
 
@@ -19,16 +20,18 @@ const documentation = `
 </p>
 `
 
+const queryParamSchema = Joi.object({
+  compact_message: Joi.equal(''),
+  passed_label: Joi.string(),
+  failed_label: Joi.string(),
+  skipped_label: Joi.string(),
+}).required()
+
 module.exports = class AppVeyorTests extends AppVeyorBase {
   static get route() {
     return {
       ...this.buildRoute('appveyor/tests'),
-      queryParams: [
-        'compact_message',
-        'passed_label',
-        'failed_label',
-        'skipped_label',
-      ],
+      queryParamSchema,
     }
   }
 
diff --git a/services/appveyor/appveyor-tests.tester.js b/services/appveyor/appveyor-tests.tester.js
index cfd2fbb6b1..25acacd4a3 100644
--- a/services/appveyor/appveyor-tests.tester.js
+++ b/services/appveyor/appveyor-tests.tester.js
@@ -1,6 +1,7 @@
 'use strict'
 
 const Joi = require('joi')
+const queryString = require('querystring')
 
 const isAppveyorTestTotals = Joi.string().regex(
   /^[0-9]+ passed(, [0-9]+ failed)?(, [0-9]+ skipped)?$/
@@ -56,14 +57,14 @@ t.create('Test status with custom labels')
 
 t.create('Test status with compact message and custom labels')
   .timeout(10000)
-  .get('/NZSmartie/coap-net-iu0to.json', {
-    qs: {
+  .get(
+    `/NZSmartie/coap-net-iu0to.json?${queryString.stringify({
       compact_message: null,
       passed_label: '💃',
       failed_label: '🤦‍♀️',
       skipped_label: '🤷',
-    },
-  })
+    })}`
+  )
   .expectJSONTypes(
     Joi.object().keys({
       name: 'tests',
diff --git a/services/mozilla-observatory/mozilla-observatory.tester.js b/services/mozilla-observatory/mozilla-observatory.tester.js
index 81cce57f6e..db9f59ff13 100644
--- a/services/mozilla-observatory/mozilla-observatory.tester.js
+++ b/services/mozilla-observatory/mozilla-observatory.tester.js
@@ -18,9 +18,7 @@ t.create('request on observatory.mozilla.org')
   )
 
 t.create('request on observatory.mozilla.org with inclusion in public results')
-  .get(
-    '/grade-score/observatory.mozilla.org.json?publish=true&style=_shields_test'
-  )
+  .get('/grade-score/observatory.mozilla.org.json?publish&style=_shields_test')
   .expectJSONTypes(
     Joi.object().keys({
       name: 'observatory',