audit-logs: handle bad url-encoded X-Action-Notes header (#1268)

lib/model/query/audits.js

@@ -11,17 +11,19 @@ const { sql } = require('slonik');
 const { map, mergeLeft } = require('ramda');
 const { Actee, Actor, Audit, Dataset, Entity, Form, Project, Submission } = require('../frames');
 const { extender, sqlEquals, page, QueryOptions, unjoiner } = require('../../util/db');
+const { urlDecode } = require('../../util/http');
 const Option = require('../../util/option');
+const Problem = require('../../util/problem');
 const { construct } = require('../../util/util');
 
+const xActionNotes = 'x-action-notes';
 
 const log = (actor, action, actee, details) => ({ run, context }) => {
   const actorId = Option.of(actor).map((x) => x.id).orNull();
   const acteeId = Option.of(actee).map((x) => x.acteeId).orNull();
   const processed = Audit.actionableEvents.includes(action) ? null : sql`clock_timestamp()`;
-  const notes = (context == null) ? null :
-    context.headers['x-action-notes'] == null ? null :
-    decodeURIComponent(context.headers['x-action-notes']); // eslint-disable-line indent
+
+  const notes = Option.of(context?.headers[xActionNotes]).map(val => urlDecode(val).orThrow(Problem.user.invalidHeader(xActionNotes))).orNull();
 
   return run(sql`
 insert into audits ("actorId", action, "acteeId", details, notes, "loggedAt", processed, failures)

lib/util/problem.js

@@ -14,6 +14,7 @@
 // A list of our Problems with their codes and messages can be found below.
 
 const { floor } = Math;
+const { ifElse, is } = require('ramda');
 const { printPairs } = require('./util');
 
 class Problem extends Error {
@@ -65,7 +66,11 @@ const problems = {
     // 400.5 (uniquenessViolation), was used in the past, but has been deprecated
 
     // { header: "the problematic header", value: "the supplied (problematic) value" }
-    invalidHeader: problem(400.6, ({ field, value }) => `An expected header field (${field}) did not match the expected format (got: ${(value == null) ? '[nothing]' : value}).`),
+    invalidHeader: ifElse(
+      is(String),
+      field => new Problem(400.6, `An expected header field (${field}) did not match the expected format.`, { field }),
+      problem(400.6, ({ field, value }) => `An expected header field (${field}) did not match the expected format (got: ${(value == null) ? '[nothing]' : value}).`),
+    ),
 
     // { field: "the name of the missing field" }
     missingMultipartField: problem(400.7, ({ field }) => `Required multipart POST field ${field} missing.`),

test/integration/api/audits.js

@@ -713,6 +713,21 @@ describe('/audits', () => {
               body[3].action.should.equal('user.session.create');
             })))));
 
+    it('should fail gracefully if note decoding fails', testService((service) =>
+      service.login('alice', (asAlice) =>
+        asAlice.post('/v1/projects/1/forms?publish=true')
+          .set('Content-Type', 'application/xml')
+          .set('X-Action-Notes', 'doing this for fun%ae')
+          .send(testData.forms.binaryType)
+          .expect(400)
+          .then(({ body }) => {
+            body.should.deepEqual({
+              code: 400.6,
+              details: { field: 'x-action-notes' },
+              message: 'An expected header field (x-action-notes) did not match the expected format.',
+            });
+          }))));
+
     describe('audit logs of deleted and purged actees', () => {
       it('should get the information of a purged actee', testService(async (service, { Forms }) =>
         service.login('alice', (asAlice) =>