feat(frontend): fix artifact cross namespace IDOR vuln

frontend/Dockerfile

@@ -1,7 +1,7 @@
 # Pass NODE_VERSION from frontend/.nvmrc when building this image.
 ARG NODE_VERSION
 ARG BASE_IMAGE=alpine
 FROM node:${NODE_VERSION}-slim AS build
 
 ARG COMMIT_HASH
 ENV COMMIT_HASH=${COMMIT_HASH}
@@ -10,7 +10,7 @@
 
 ARG DATE
 
 WORKDIR ./src
 
 COPY . .
 
@@ -29,7 +29,7 @@
     echo ${DATE} > ./server/dist/BUILD_DATE && \
     echo ${TAG_NAME} > ./server/dist/TAG_NAME
 
 FROM node:${NODE_VERSION}-${BASE_IMAGE}
 
 COPY --from=build ./src/frontend/package.json /tmp/frontend-package.json
 RUN npm install --global "$(node -p 'require("/tmp/frontend-package.json").packageManager')" && \
@@ -37,6 +37,9 @@
 
 COPY --from=build ./src/frontend/server /server
 COPY --from=build ./src/frontend/build /client
+# MLMD proto bundle for the runtime validator; marker forces CJS under type: module.
+COPY --from=build ./src/frontend/src/third_party/mlmd/generated /server/src/third_party/mlmd/generated
+RUN printf '{"type":"commonjs"}\n' > /server/src/third_party/mlmd/generated/package.json
 
 WORKDIR /server
 
@@ -45,5 +48,5 @@
 # race condition where parallel download workers collide on the same cache entry.
 RUN npm ci || (npm cache clean --force && npm ci)
 RUN npm run build
 ENV API_SERVER_ADDRESS http://localhost:3001
 CMD node dist/server.js ../client/ 3000

frontend/server/app.ts

@@ -168,6 +168,7 @@ function createUIServer(options: UIConfigs) {
     authorizeFn,
     options.auth.enabled,
     options.auth.kubeflowUserIdHeader,
+    envoyServiceAddress,
   );
 
   /** Artifact */

frontend/server/handlers/artifacts.test.ts

@@ -0,0 +1,195 @@
+// Copyright 2025 The Kubeflow Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+import { describe, it, expect } from 'vitest';
+import type { Request } from 'express';
+import { resolveArtifactCoordinates } from '../helpers/artifact-coordinates.js';
+
+function makeRequest(path: string, query: Record<string, unknown> = {}): Request {
+  return { path, query } as unknown as Request;
+}
+
+describe('resolveArtifactCoordinates', () => {
+  describe('path-based routes', () => {
+    it('extracts coordinates from /artifacts/:source/:bucket/* style URLs', () => {
+      const req = makeRequest('/artifacts/minio/ml-pipeline/hello/world.txt');
+      expect(resolveArtifactCoordinates(req)).toEqual({
+        source: 'minio',
+        bucket: 'ml-pipeline',
+        key: 'hello/world.txt',
+      });
+    });
+
+    it('extracts coordinates from /pipeline-prefixed routes', () => {
+      const req = makeRequest('/pipeline/artifacts/s3/my-bucket/path/to/file.csv');
+      expect(resolveArtifactCoordinates(req)).toEqual({
+        source: 's3',
+        bucket: 'my-bucket',
+        key: 'path/to/file.csv',
+      });
+    });
+
+    it('extracts coordinates from a non-/pipeline base path', () => {
+      const req = makeRequest('/foo/bar/artifacts/minio/my-bucket/some/key');
+      expect(resolveArtifactCoordinates(req)).toEqual({
+        source: 'minio',
+        bucket: 'my-bucket',
+        key: 'some/key',
+      });
+    });
+
+    it('decodes percent-encoded path segments once (matching Express req.params semantics)', () => {
+      const req = makeRequest('/artifacts/minio/ml-pipeline/hello%2Fworld.txt');
+      expect(resolveArtifactCoordinates(req)).toEqual({
+        source: 'minio',
+        bucket: 'ml-pipeline',
+        key: 'hello/world.txt',
+      });
+    });
+
+    it('preserves a literal %2F when the URL is double-encoded (%252F)', () => {
+      const req = makeRequest('/artifacts/minio/ml-pipeline/hello%252Fworld.txt');
+      expect(resolveArtifactCoordinates(req)).toEqual({
+        source: 'minio',
+        bucket: 'ml-pipeline',
+        key: 'hello%2Fworld.txt',
+      });
+    });
+
+    it('returns null on malformed percent-encoding (fail-closed)', () => {
+      const req = makeRequest('/artifacts/minio/ml-pipeline/bad%ZZkey');
+      expect(resolveArtifactCoordinates(req)).toBeNull();
+    });
+
+    it('handles keys that contain multiple slashes', () => {
+      const req = makeRequest('/artifacts/gcs/my-bucket/a/b/c/d.json');
+      expect(resolveArtifactCoordinates(req)).toEqual({
+        source: 'gcs',
+        bucket: 'my-bucket',
+        key: 'a/b/c/d.json',
+      });
+    });
+  });
+
+  describe('query-based fallback (/artifacts/get and unrecognized paths)', () => {
+    it('falls back to query when path is /artifacts/get', () => {
+      const req = makeRequest('/artifacts/get', {
+        source: 'minio',
+        bucket: 'ml-pipeline',
+        key: 'hello/world.txt',
+      });
+      expect(resolveArtifactCoordinates(req)).toEqual({
+        source: 'minio',
+        bucket: 'ml-pipeline',
+        key: 'hello/world.txt',
+      });
+    });
+
+    it('falls back to query when path is /pipeline/artifacts/get', () => {
+      const req = makeRequest('/pipeline/artifacts/get', {
+        source: 's3',
+        bucket: 'my-bucket',
+        key: 'data.csv',
+      });
+      expect(resolveArtifactCoordinates(req)).toEqual({
+        source: 's3',
+        bucket: 'my-bucket',
+        key: 'data.csv',
+      });
+    });
+
+    it('falls back to query when path does not match the artifact patterns', () => {
+      const req = makeRequest('/foo/bar', {
+        source: 'minio',
+        bucket: 'ml-pipeline',
+        key: 'k',
+      });
+      expect(resolveArtifactCoordinates(req)).toEqual({
+        source: 'minio',
+        bucket: 'ml-pipeline',
+        key: 'k',
+      });
+    });
+  });
+
+  describe('missing or non-string query values', () => {
+    it('returns empty strings when query params are absent', () => {
+      const req = makeRequest('/artifacts/get');
+      expect(resolveArtifactCoordinates(req)).toEqual({
+        source: '',
+        bucket: '',
+        key: '',
+      });
+    });
+
+    it('rejects array-valued query params (treats them as missing)', () => {
+      const req = makeRequest('/artifacts/get', {
+        source: ['minio', 'sneaky'],
+        bucket: 'ml-pipeline',
+        key: 'k',
+      });
+      expect(resolveArtifactCoordinates(req)).toEqual({
+        source: '',
+        bucket: 'ml-pipeline',
+        key: 'k',
+      });
+    });
+
+    it('rejects object-valued query params (treats them as missing)', () => {
+      const req = makeRequest('/artifacts/get', {
+        source: { nested: 'minio' },
+        bucket: 'ml-pipeline',
+        key: 'k',
+      });
+      expect(resolveArtifactCoordinates(req)).toEqual({
+        source: '',
+        bucket: 'ml-pipeline',
+        key: 'k',
+      });
+    });
+  });
+
+  describe('coordinate-source spoofing defense', () => {
+    it('uses path coordinates when both path and query are present', () => {
+      // Attacker plants benign values in query, real values in path.
+      const req = makeRequest('/artifacts/minio/victim-bucket/secret.txt', {
+        source: 'minio',
+        bucket: 'safe-bucket',
+        key: 'safe-key',
+      });
+      expect(resolveArtifactCoordinates(req)).toEqual({
+        source: 'minio',
+        bucket: 'victim-bucket',
+        key: 'secret.txt',
+      });
+    });
+
+    it('treats /artifacts/get/x/y as a path-based route with source=get (not the get endpoint)', () => {
+      // Only an exact /artifacts/get path uses the query string; any other
+      // path that matches /:source/:bucket/* uses path values, even if the
+      // first segment happens to literally be "get". The downstream handler
+      // then rejects source="get" as an unknown storage source (500).
+      const req = makeRequest('/artifacts/get/some-bucket/some-key', {
+        source: 'minio',
+        bucket: 'safe-bucket',
+        key: 'safe-key',
+      });
+      expect(resolveArtifactCoordinates(req)).toEqual({
+        source: 'get',
+        bucket: 'some-bucket',
+        key: 'some-key',
+      });
+    });
+  });
+});

frontend/server/handlers/artifacts.ts

@@ -41,6 +41,8 @@ import { isAllowedDomain } from './domain-checker.js';
 import { getK8sSecret } from '../k8s-helper.js';
 import { CredentialBody } from 'google-auth-library';
 import { AuthorizeFn } from '../helpers/auth.js';
+import { validateArtifactNamespace, buildArtifactUri } from '../helpers/mlmd-validator.js';
+import { resolveArtifactCoordinates } from '../helpers/artifact-coordinates.js';
 import {
   AuthorizeRequestResources,
   AuthorizeRequestVerb,
@@ -121,11 +123,14 @@ export interface GCSProviderInfo {
  * @param authorizeFn The authorization function to validate permissions
  * @param authEnabled Whether authorization is enabled
  * @param kubeflowUserIdHeader The header name containing the user identity
+ * @param envoyAddress MLMD Envoy address used for namespace-ownership
+ *   validation (#9889). When omitted, the IDOR check is skipped.
  */
 export function getArtifactsAuthMiddleware(
   authorizeFn: AuthorizeFn,
   authEnabled: boolean,
   kubeflowUserIdHeader: string,
+  envoyAddress?: string,
 ): Handler {
   return async (request: Request, response: Response, next: NextFunction) => {
     if (!authEnabled) {
@@ -187,6 +192,36 @@ export function getArtifactsAuthMiddleware(
       return;
     }
 
+    if (envoyAddress) {
+      const coords = resolveArtifactCoordinates(request);
+      if (coords === null) {
+        console.warn(
+          `[SECURITY] Malformed percent-encoding in artifact path. ` +
+            `User: ${userId}, Path: ${request.path}`,
+        );
+        response.status(400).send('Malformed URL encoding in artifact path');
+        return;
+      }
+      const mlmdTrackedSources = new Set(['minio', 's3', 'gcs', 'http', 'https']);
+      if (mlmdTrackedSources.has(coords.source) && coords.bucket && coords.key) {
+        const artifactUri = buildArtifactUri(coords.source, coords.bucket, coords.key);
+        const validation = await validateArtifactNamespace(envoyAddress, artifactUri, namespace);
+
+        if (!validation.valid) {
+          console.warn(
+            `[SECURITY] IDOR blocked: artifact namespace mismatch. ` +
+              `User: ${userId}, ` +
+              `Claimed namespace: ${namespace}, ` +
+              `Actual namespace: ${validation.actualNamespace}, ` +
+              `URI: ${artifactUri}, ` +
+              `Path: ${request.path}`,
+          );
+          response.status(403).send('Artifact does not belong to the requested namespace');
+          return;
+        }
+      }
+    }
+
     next();
   };
 }

frontend/server/helpers/artifact-coordinates.ts

@@ -0,0 +1,44 @@
+// Copyright 2025 The Kubeflow Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+import type { Request } from 'express';
+
+export function resolveArtifactCoordinates(
+  request: Request,
+): { source: string; bucket: string; key: string } | null {
+  const artifactPathStart = request.path.indexOf('/artifacts/');
+  const artifactPath =
+    artifactPathStart >= 0 ? request.path.slice(artifactPathStart) : request.path;
+  const isExactGetEndpoint = artifactPath === '/artifacts/get';
+  if (!isExactGetEndpoint) {
+    const downloadPathMatch = artifactPath.match(/^\/artifacts\/([^/]+)\/([^/]+)\/(.+)$/);
+    if (downloadPathMatch) {
+      try {
+        return {
+          source: decodeURIComponent(downloadPathMatch[1]),
+          bucket: decodeURIComponent(downloadPathMatch[2]),
+          key: decodeURIComponent(downloadPathMatch[3]),
+        };
+      } catch {
+        return null;
+      }
+    }
+  }
+  const asString = (v: unknown): string => (typeof v === 'string' ? v : '');
+  return {
+    source: asString(request.query.source),
+    bucket: asString(request.query.bucket),
+    key: asString(request.query.key),
+  };
+}