Merge pull request #219 from bcgov/bugfix/metadata-length

Remove Metadata length constraint and update dependencies

Author: TimCsaky

Dockerfile

@@ -7,7 +7,7 @@
 #
 # Build the application
 #
-FROM registry.access.redhat.com/ubi9/nodejs-18:1-62.1692771036 as builder
+FROM registry.access.redhat.com/ubi9/nodejs-18:1-70.1695740477 as builder
 
 ENV NO_UPDATE_NOTIFIER=true
 
@@ -22,7 +22,7 @@ RUN npm ci --omit=dev
 #
 # Create the final container image
 #
-FROM registry.access.redhat.com/ubi9/nodejs-18-minimal:1-67
+FROM registry.access.redhat.com/ubi9/nodejs-18-minimal:1-74.1695740475
 
 ENV APP_PORT=3000 \
     NO_UPDATE_NOTIFIER=true

app/package-lock.json

@@ -29,10 +29,10 @@
     "seed": "knex seed:run"
   },
   "dependencies": {
-    "@aws-sdk/client-s3": "^3.388.0",
-    "@aws-sdk/lib-storage": "^3.388.0",
-    "@aws-sdk/s3-request-presigner": "^3.388.0",
-    "api-problem": "^9.0.1",
+    "@aws-sdk/client-s3": "^3.423.0",
+    "@aws-sdk/lib-storage": "^3.423.0",
+    "@aws-sdk/s3-request-presigner": "^3.423.0",
+    "api-problem": "^9.0.2",
     "compression": "^1.7.4",
     "config": "^3.3.9",
     "content-disposition": "^0.5.4",
@@ -41,22 +41,22 @@
     "express": "^4.18.2",
     "express-basic-auth": "^1.2.1",
     "express-winston": "^4.2.0",
-    "joi": "^17.10.2",
+    "joi": "^17.11.0",
     "js-yaml": "^4.1.0",
-    "jsonwebtoken": "^9.0.1",
+    "jsonwebtoken": "^9.0.2",
     "knex": "^2.5.1",
-    "objection": "^3.1.1",
-    "pg": "^8.11.2",
+    "objection": "^3.1.2",
+    "pg": "^8.11.3",
     "winston": "^3.10.0",
     "winston-transport": "^4.5.0"
   },
   "devDependencies": {
     "aws-sdk-client-mock": "^3.0.0",
     "aws-sdk-client-mock-jest": "^3.0.0",
-    "eslint": "^8.47.0",
+    "eslint": "^8.50.0",
     "eslint-config-recommended": "^4.1.0",
     "eslint-plugin-prettier": "^5.0.0",
-    "jest": "~29.3.1",
+    "jest": "^29.7.0",
     "jest-joi": "^1.1.17",
     "nodemon": "^3.0.1",
     "supertest": "^6.3.3"

app/package.json

@@ -277,14 +277,15 @@ const utils = {
 
   /**
    * @function isAtPath
-   * Predicate function determining if the `path` is a member of the `prefix` path
+   * Predicate function determining if the `path` is a non-directory member of the `prefix` path
    * @param {string} prefix The base "folder"
    * @param {string} path The "file" to check
    * @returns {boolean} True if path is member of prefix. False in all other cases.
    */
   isAtPath(prefix, path) {
     if (typeof prefix !== 'string' || typeof path !== 'string') return false;
     if (prefix === path) return true; // Matching strings are always at the at the path
+    if (path.endsWith(DELIMITER)) return false; // Trailing slashes references the folder
 
     const pathParts = path.split(DELIMITER).filter(part => part);
     const prefixParts = prefix.split(DELIMITER).filter(part => part);

app/src/components/utils.js

@@ -0,0 +1,17 @@
+exports.up = function (knex) {
+  return Promise.resolve()
+    // Change to text type
+    .then(() => knex.schema.alterTable('metadata', table => {
+      table.text('key').notNullable().alter();
+      table.text('value').notNullable().alter();
+    }));
+};
+
+exports.down = function (knex) {
+  return Promise.resolve()
+    // Revert back to varchar(255) type
+    .then(() => knex.schema.alterTable('metadata', table => {
+      table.string('key', 255).notNullable().alter();
+      table.string('value', 255).notNullable().alter();
+    }));
+};

app/src/db/migrations/20231010000000_011-metadata-text.js

@@ -76,8 +76,8 @@ class Metadata extends Model {
       required: ['key', 'value'],
       properties: {
         id: { type: 'integer' },
-        key: { type: 'string', minLength: 1, maxLength: 255 },
-        value: { type: 'string', minLength: 1, maxLength: 255 }
+        key: { type: 'string', minLength: 1 },
+        value: { type: 'string', minLength: 1 }
       },
       additionalProperties: false
     };

app/src/db/models/tables/metadata.js

@@ -1477,10 +1477,10 @@ components:
       description: >-
         An arbitrary metadata key/value pair. Must contain the x-amz-meta-
         prefix to be valid. Multiple metadata pairs can be defined. Keys must be
-        unique and will be converted to lowercase.
+        unique, contain no whitespace, and will be converted to lowercase.
       schema:
         type: string
-        maximum: 255
+        minimum: 1
         example:
           - x-amz-meta-foo
           - x-amz-meta-bar

app/src/docs/v1.api-spec.yaml

@@ -50,8 +50,9 @@ const type = {
    * @param {number} [options.minValueStringLength=1] Optional minimum string length of metadata value allowed,
    * @returns {object} Joi object
    */
+  // TODO: Simplify by changing from arrow function to property
   metadata: ({ minKeyCount = 0, minValueStringLength = 1 } = {}) => Joi.object()
-    .pattern(/^x-amz-meta-.{1,255}$/i, Joi.string().min(minValueStringLength).max(255), { matches: Joi.array().min(minKeyCount) })
+    .pattern(/^x-amz-meta-\S+$/i, Joi.string().min(minValueStringLength), { matches: Joi.array().min(minKeyCount) })
     .unknown(),
 
   /**
@@ -63,6 +64,7 @@ const type = {
    * (default of 9 because COMS also adds a `coms-id` tag by default)
    * @returns {object} Joi object
    */
+  // TODO: Simplify by changing from arrow function to property
   tagset: ({ maxKeyCount = 9, minKeyCount = 0, minValueStringLength = 0 } = {}) => Joi.object()
     .pattern(
       /^(?!coms-id$).{1,255}$/, // don't allow key 'coms-id'

app/src/validators/common.js

@@ -396,9 +396,11 @@ describe('isAtPath', () => {
     [true, '/foo', 'foo/bar'],
     [true, '/foo', '/foo/bar'],
     [true, 'a/b', 'a/b/foo.jpg'],
+    [false, 'a', 'a/b/'], // Trailing slashes references the folder and should be excluded
     [false, 'a/b', 'a/b/'], // Trailing slashes references the folder and should be excluded
     [false, 'a/b', 'a/b/z/deep.jpg'],
     [false, 'a/b', 'a/b/y/z/deep.jpg'],
+    [false, 'a/b', 'a/b/c/'], // Trailing slashes references the folder and should be excluded
     [false, 'a/b/c', 'a/b/c/'], // Trailing slashes references the folder and should be excluded
     [false, 'a/b/c', 'a/bar.png'],
     [false, 'c/b/a', 'a/b/c/bar.png'],

app/tests/unit/components/utils.spec.js

@@ -147,7 +147,7 @@ describe('type', () => {
     it('enforces general metadata pattern', () => {
       expect(model.patterns).toEqual(expect.arrayContaining([
         expect.objectContaining({
-          regex: '/^x-amz-meta-.{1,255}$/i',
+          regex: '/^x-amz-meta-\\S+$/i',
           rule: expect.objectContaining({
             type: 'string',
             rules: expect.arrayContaining([
@@ -156,12 +156,6 @@ describe('type', () => {
                 args: expect.objectContaining({
                   limit: 1
                 })
-              }),
-              expect.objectContaining({
-                name: 'max',
-                args: expect.objectContaining({
-                  limit: 255
-                })
               })
             ])
           })