add valid key checks

Signed-off-by: JerT33 <[email protected]>

Author: JerT33

frontend/server/handlers/artifacts.ts

@@ -19,6 +19,7 @@ import {
   findFileOnPodVolume,
   parseJSONString,
   isAllowedResourceName,
+  isAllowedObjectKey,
 } from '../utils';
 import { createMinioClient, getObjectStream } from '../minio-helper';
 import * as serverInfo from '../helpers/server-info';
@@ -124,6 +125,10 @@ export function getArtifactsHandler({
       res.status(500).send('Storage key is missing from artifact request');
       return;
     }
+    if (!isAllowedObjectKey(key)) {
+      res.status(400).send('Invalid object key');
+      return;
+    }
     console.log(`Getting storage artifact at: ${source}: ${bucket}/${key}`);
 
     let client: MinioClient;

frontend/server/integration-tests/artifact-get.test.ts

@@ -838,6 +838,30 @@ describe('/artifacts', () => {
         .get(`/artifacts/get?source=volume&bucket=artifact&key=subartifact/notxist.csv`)
         .expect(500, 'Failed to open volume.', done);
     });
+
+    it('rejects keys with encoded XSS payloads', done => {
+      const configs = loadConfigs(argv, {
+        AWS_ACCESS_KEY_ID: 'aws123',
+        AWS_SECRET_ACCESS_KEY: 'awsSecret123',
+      });
+      app = new UIServer(configs);
+      const request = requests(app.start());
+      request
+        .get('/artifacts/get?source=s3&namespace=test&peek=256&bucket=ml-pipeline&key=%3Cscript%3Ealert(1)%3C%2Fscript%3E')
+        .expect(400, 'Invalid object key', done);
+    });
+
+    it('rejects keys with repeated unsafe characters', done => {
+      const configs = loadConfigs(argv, {
+        AWS_ACCESS_KEY_ID: 'aws123',
+        AWS_SECRET_ACCESS_KEY: 'awsSecret123',
+      });
+      app = new UIServer(configs);
+      const request = requests(app.start());
+      request
+        .get('/artifacts/get?source=s3&namespace=test&peek=256&bucket=ml-pipeline&key=' + '%2b['.repeat(300))
+        .expect(400, 'Invalid object key', done);
+    });
   });
 
   describe('/:source/:bucket/:key', () => {

frontend/server/utils.ts

@@ -304,3 +304,7 @@ function parseK8sError(error: any): ErrorDetails | undefined {
 export function isAllowedResourceName(name: string): boolean {
   return name.length > 0 && name.length <= 63 && /^[a-z0-9]([-a-z0-9]*[a-z0-9])?$/.test(name);
 }
+
+export function isAllowedObjectKey(key: string): boolean {
+  return key.length > 0 && key.length <= 1024 && /^[a-zA-Z0-9\/\-_.]+$/.test(key);
+}