This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-7937-b01b11f8182313b9f3bff877c661f7ae0f0b5a1c in repository https://gitbox.apache.org/repos/asf/texera.git
commit 1996448b1c5f4e0906cc077f927a90648e28a1af Author: Tanishq Gandhi <[email protected]> AuthorDate: Thu Aug 27 19:50:49 2026 +0000 feat(file-service): complete the model backend for sharing and the hub (#7937) ### What changes were proposed in this PR? #7922 gave models the file API the management UI needs. This PR adds the last backend pieces, so model sharing and the hub can be built as frontend-only work. **Cover images** — `POST /{mid}/update/cover`, `GET /{mid}/cover` (307 redirect), `GET /{mid}/cover-url` (JSON, since `<img src>` cannot attach the Authorization header on a private model). The `cover_image` column already existed. The resource-agnostic halves move into a shared `CoverImageUtils` and `DatasetResource` is refactored onto it, net **−55 lines** there. The image extension allowlist is a security control, not a convention — a cover is handed to the browser as a presigned URL, so an active document (`.svg`, `.html`) would be a stored-XSS vector, and a duplicated allowlist drifts. A cover path is validated to be `<version>/<file>` and opened defensively. `FileResolver` needs five path segments and throws an `IOException` that file-service registers no mapper for, so a bare file name or a deleted version would otherwise surface as an opaque 500 — and on the read path, one bad write would 500 every render of every card showing that resource. The write path answers 400; the reads treat an unresolvable cover as absent (`{"url": null}` / 404) so the card falls back to its default. **Anonymous version browsing** — `/{mid}/publicVersion/list` and `/{mid}/publicVersion/{mvid}/rootFileNodes`. Without these a logged-out visitor on a public model page gets metadata and then an error. **Framework/format editing** — `POST /update/framework`, `POST /update/format`, plus `other` in both whitelists. The labels were create-only, so a mislabelled model had to be recreated. Both endpoints share `normalizeLabel` with `createModel`, so the edit path cannot reject a value the create path accepted: blank resets the framework to the default and clears the format, and values are trimmed the same way in both. **Routing** — `/api/model` and `/api/access/model` in nginx, the k8s gateway and the frontend dev proxy. Nothing reaches the resource in a real deployment without them. The nginx location carries a trailing slash for symmetry with `/api/access/dataset/`; the LLM `/api/models` route is an exact-match location, which outranks any prefix location, so the two are unambiguous either way. One behavior change on the dataset side: covers now get a path-length check. `dataset.cover_image` is `varchar(246)` via `sql/updates/18.sql`, so an over-long path was a jOOQ-wrapped 500 and is now a 400. The dataset cover endpoints also pick up the unresolvable-path handling above, which they lacked before this branch. ### Any related issues, documentation, discussions? - Part of #6501 - **Reviewable on its own.** #7922 has merged and `main` has been merged in, so the diff is this PR's work alone. No migration, no schema change. - Independent of #7930 (models in the hub and unified search). That PR is amber-side — the dashboard search builders, `EntityTables`/`EntityType`, and `sql/42.sql`; the `ModelResource` it touches is the amber dashboard one, not file-service's. Neither PR calls the other, and this branch compiles and passes its tests on `main` with none of #7930 present. They can merge in either order; whichever lands second will hit a one-line conflict in `ModelResourcePermissionsSpec`'s `publicEndpointMethods` set. ### How was this PR tested? New `CoverImageUtilsSpec` pins the allowlist, `.svg`/`.html` rejection, path traversal, absolute paths, the `<version>/<file>` shape and both column-width limits. New `ModelHubApiSpec` drives the covers end to end through LakeFS/MinIO and covers anonymous browsing, including the negative cases: private model anonymous and no-grant, unknown model and version id, a model unpublished after the fact, a bare filename and a nonexistent version rejected at write, a stored cover that stops resolving, and agreement between the public and authenticated file-tree endpoints. `ModelApiForUiSpec` covers the framework/format updates, including trimming, blank-resets-framework and blank-clears-format. `ModelResourcePermissionsSpec` needed the four new `@PermitAll` endpoint names — the guardrail caught them, as intended. The dataset suites pass unmodified, which is what confirms the shared `CoverImageUtils` did not shift dataset behavior. 300 tests across the eight affected suites: sbt "FileService/testOnly *CoverImageUtilsSpec *ModelHubApiSpec *ModelApiForUiSpec *ModelResourcePermissionsSpec *ModelResourceSpec *DatasetResourceSpec *DatasetResourcePermissionsSpec *DatasetAccessResourceSpec" sbt scalafmtCheckAll "scalafixAll --check" ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (Claude Opus 5) --------- Co-authored-by: ali <[email protected]> Co-authored-by: Claude Opus 4.8 <[email protected]> --- bin/k8s/templates/base/gateway/gateway-routes.yaml | 8 + bin/single-node/nginx.conf | 15 + .../texera/service/resource/DatasetResource.scala | 154 +++----- .../texera/service/resource/ModelResource.scala | 244 ++++++++++++- .../texera/service/util/CoverImageUtils.scala | 134 +++++++ .../service/resource/DatasetResourceSpec.scala | 15 +- .../service/resource/ModelApiForUiSpec.scala | 124 +++++++ .../texera/service/resource/ModelHubApiSpec.scala | 398 +++++++++++++++++++++ .../resource/ModelResourcePermissionsSpec.scala | 10 +- .../texera/service/util/CoverImageUtilsSpec.scala | 158 ++++++++ frontend/proxy.config.json | 10 + 11 files changed, 1134 insertions(+), 136 deletions(-) diff --git a/bin/k8s/templates/base/gateway/gateway-routes.yaml b/bin/k8s/templates/base/gateway/gateway-routes.yaml index 935d38bb07..f07a3157a4 100644 --- a/bin/k8s/templates/base/gateway/gateway-routes.yaml +++ b/bin/k8s/templates/base/gateway/gateway-routes.yaml @@ -49,6 +49,14 @@ spec: - path: type: PathPrefix value: /api/access/dataset + # PathPrefix matches whole path segments, so /api/model and the /api/models + # LLM route below stay distinct. + - path: + type: PathPrefix + value: /api/model + - path: + type: PathPrefix + value: /api/access/model backendRefs: - name: file-service-svc port: 9092 diff --git a/bin/single-node/nginx.conf b/bin/single-node/nginx.conf index 0f091a5cb4..688e256d46 100644 --- a/bin/single-node/nginx.conf +++ b/bin/single-node/nginx.conf @@ -45,6 +45,21 @@ http { proxy_set_header X-Real-IP $remote_addr; } + # Trailing slash for symmetry with /api/access/dataset/ above. The LLM route + # is an exact match (location = /api/models), which outranks any prefix + # location, so /api/models is unambiguous either way. + location /api/model/ { + proxy_pass http://file-service:9092; + proxy_set_header Host $host; + proxy_set_header X-Real-IP $remote_addr; + } + + location /api/access/model/ { + proxy_pass http://file-service:9092; + proxy_set_header Host $host; + proxy_set_header X-Real-IP $remote_addr; + } + location /api/config { proxy_pass http://config-service:9094; proxy_set_header Host $host; diff --git a/file-service/src/main/scala/org/apache/texera/service/resource/DatasetResource.scala b/file-service/src/main/scala/org/apache/texera/service/resource/DatasetResource.scala index f8bc532909..6c15196a0b 100644 --- a/file-service/src/main/scala/org/apache/texera/service/resource/DatasetResource.scala +++ b/file-service/src/main/scala/org/apache/texera/service/resource/DatasetResource.scala @@ -26,9 +26,8 @@ import jakarta.ws.rs._ import jakarta.ws.rs.core._ import org.apache.texera.common.config.StorageConfig import org.apache.texera.common.util.EmailUtil -import org.apache.texera.amber.core.storage.model.OnVersionedFileResource import org.apache.texera.amber.core.storage.util.LakeFSStorageClient -import org.apache.texera.amber.core.storage.{DocumentFactory, FileResolver, ResourceType} +import org.apache.texera.amber.core.storage.ResourceType import org.apache.texera.auth.SessionUser import org.apache.texera.dao.SiteSettings import org.apache.texera.dao.SqlServer @@ -52,6 +51,8 @@ import org.apache.texera.service.`type`.{Diff, ExistingUploadFilesRequest, LakeF import org.apache.texera.service.resource.DatasetAccessResource._ import org.apache.texera.service.resource.ResourceTables.{Dataset => DATASET_RESOURCE} import org.apache.texera.service.resource.DatasetResource.{context, _} +import org.apache.texera.service.util.CoverImageUtils +import org.apache.texera.service.util.CoverImageUtils.CoverImageRequest import org.apache.texera.service.util.S3StorageClient import org.jooq.impl.DSL import org.jooq.{DSLContext, EnumType} @@ -62,7 +63,6 @@ import java.util import java.util.Optional import scala.jdk.CollectionConverters._ import scala.jdk.OptionConverters._ -import org.apache.commons.io.FilenameUtils import org.apache.texera.service.util.LakeFSExceptionHandler.withLakeFSErrorHandling object DatasetResource { @@ -72,15 +72,6 @@ object DatasetResource { .getInstance() .createDSLContext() - // Builds a resource logical path (/<resourceType>/ownerEmail/resourceName/relativePath). - private def logicalPath( - resourceType: ResourceType.Value, - ownerEmail: String, - resourceName: String, - relativePath: String - ): String = - s"$resourceType/$ownerEmail/$resourceName/$relativePath" - private def singleFileUploadMaxBytes(defaultMiB: Long = 20L): Long = SiteSettings.getLong("single_file_upload_max_size_mib", defaultMiB) * 1024L * 1024L @@ -271,8 +262,6 @@ object DatasetResource { fileNodes: List[LakeFSFileNode], size: Long ) - - case class CoverImageRequest(coverImage: String) } @Produces(Array(MediaType.APPLICATION_JSON, "image/jpeg", "application/pdf")) @@ -282,26 +271,39 @@ class DatasetResource extends LazyLogging { private val ERR_DATASET_VERSION_NOT_FOUND_MESSAGE = "The version of the dataset not found" private val EXPIRATION_MINUTES = 5 - private val COVER_IMAGE_SIZE_LIMIT_BYTES: Long = 10 * 1024 * 1024 // 10 MB - private val ALLOWED_IMAGE_EXTENSIONS: Set[String] = Set(".jpg", ".jpeg", ".png", ".gif", ".webp") + // varchar(246) on DBs upgraded through sql/updates/18.sql, narrower than model's. + private val COVER_IMAGE_MAX_PATH_LENGTH = 246 private val resourceType = ResourceType.Dataset /** - * Helper function to get the dataset from DB with additional information including user access privilege and owner email + * The single read rule for a dataset: anonymous callers get public datasets only, a + * signed-in caller goes through userHasReadAccess. Shared with getDashboardDataset + * so the cover endpoints cannot drift from it. */ - private def getDashboardDataset( + private def requireReadAccess( ctx: DSLContext, did: Integer, requesterUid: Option[Integer] - ): DashboardDataset = { - val targetDataset = getDatasetByID(ctx, did) - - if (requesterUid.isEmpty && !targetDataset.getIsPublic) { + ): Dataset = { + val dataset = getDatasetByID(ctx, did) + if (requesterUid.isEmpty && !dataset.getIsPublic) { throw new ForbiddenException(ERR_USER_HAS_NO_ACCESS_TO_DATASET_MESSAGE) } else if (requesterUid.exists(uid => !userHasReadAccess(ctx, did, uid))) { throw new ForbiddenException(ERR_USER_HAS_NO_ACCESS_TO_DATASET_MESSAGE) } + dataset + } + + /** + * Helper function to get the dataset from DB with additional information including user access privilege and owner email + */ + private def getDashboardDataset( + ctx: DSLContext, + did: Integer, + requesterUid: Option[Integer] + ): DashboardDataset = { + val targetDataset = requireReadAccess(ctx, did, requesterUid) val userAccessPrivilege = requesterUid .map(uid => getDatasetUserAccessPrivilege(ctx, did, uid)) @@ -1188,39 +1190,16 @@ class DatasetResource extends LazyLogging { throw new ForbiddenException(ERR_USER_HAS_NO_ACCESS_TO_DATASET_MESSAGE) } - if (request.coverImage == null || request.coverImage.trim.isEmpty) { - throw new BadRequestException("Cover image path is required") - } - - val normalized = ResourceNaming.validateAndNormalizeFilePathOrThrow(request.coverImage) - - val extension = FilenameUtils.getExtension(normalized) - if (extension == null || !ALLOWED_IMAGE_EXTENSIONS.contains(s".$extension".toLowerCase)) { - throw new BadRequestException("Invalid file type") - } - - val owner = getOwner(ctx, did) - val document = DocumentFactory - .openReadonlyDocument( - FileResolver.resolve( - logicalPath(resourceType, owner.getEmail, dataset.getName, normalized) - ) - ) - .asInstanceOf[OnVersionedFileResource] - - val fileSize = withLakeFSErrorHandling(s"reading the size of cover image '$normalized'") { - LakeFSStorageClient.getFileSize( - document.getRepositoryName(), - document.getVersionHash(), - document.getFileRelativePath() - ) - } + val normalized = + CoverImageUtils.validatePathOrThrow(request.coverImage, COVER_IMAGE_MAX_PATH_LENGTH) - if (fileSize > COVER_IMAGE_SIZE_LIMIT_BYTES) { - throw new BadRequestException( - s"Cover image must be less than ${COVER_IMAGE_SIZE_LIMIT_BYTES / (1024 * 1024)} MB" - ) - } + val document = CoverImageUtils.openCoverOrBadRequest( + resourceType, + getOwner(ctx, did).getEmail, + dataset.getName, + normalized + ) + CoverImageUtils.requireWithinSizeLimit(CoverImageUtils.fileSizeOf(document, normalized)) dataset.setCoverImage(normalized) new DatasetDao(ctx.configuration()).update(dataset) @@ -1243,39 +1222,19 @@ class DatasetResource extends LazyLogging { @Auth sessionUser: Optional[SessionUser] ): Response = { withTransaction(context) { ctx => - val dataset = getDatasetByID(ctx, did) - - val requesterUid = if (sessionUser.isPresent) Some(sessionUser.get().getUid) else None - - if (requesterUid.isEmpty && !dataset.getIsPublic) { - throw new ForbiddenException(ERR_USER_HAS_NO_ACCESS_TO_DATASET_MESSAGE) - } else if (requesterUid.exists(uid => !userHasReadAccess(ctx, did, uid))) { - throw new ForbiddenException(ERR_USER_HAS_NO_ACCESS_TO_DATASET_MESSAGE) - } + val dataset = requireReadAccess(ctx, did, sessionUser.toScala.map(_.getUid)) val coverImage = Option(dataset.getCoverImage).getOrElse( throw new NotFoundException("No cover image") ) - val owner = getOwner(ctx, did) - val fullPath = - logicalPath(resourceType, owner.getEmail, dataset.getName, coverImage) - - val document = DocumentFactory - .openReadonlyDocument(FileResolver.resolve(fullPath)) - .asInstanceOf[OnVersionedFileResource] + val document = CoverImageUtils + .openCover(resourceType, getOwner(ctx, did).getEmail, dataset.getName, coverImage) + .getOrElse(throw new NotFoundException("No cover image")) - val presignedUrl = withLakeFSErrorHandling( - s"generating a presigned URL for cover image '$coverImage'" - ) { - LakeFSStorageClient.getFilePresignedUrl( - document.getRepositoryName(), - document.getVersionHash(), - document.getFileRelativePath() - ) - } - - Response.temporaryRedirect(new URI(presignedUrl)).build() + Response + .temporaryRedirect(new URI(CoverImageUtils.presignedUrl(document, coverImage))) + .build() } } @@ -1293,39 +1252,16 @@ class DatasetResource extends LazyLogging { @Auth sessionUser: Optional[SessionUser] ): Response = { withTransaction(context) { ctx => - val dataset = getDatasetByID(ctx, did) - - val requesterUid = if (sessionUser.isPresent) Some(sessionUser.get().getUid) else None - - if (requesterUid.isEmpty && !dataset.getIsPublic) { - throw new ForbiddenException(ERR_USER_HAS_NO_ACCESS_TO_DATASET_MESSAGE) - } else if (requesterUid.exists(uid => !userHasReadAccess(ctx, did, uid))) { - throw new ForbiddenException(ERR_USER_HAS_NO_ACCESS_TO_DATASET_MESSAGE) - } + val dataset = requireReadAccess(ctx, did, sessionUser.toScala.map(_.getUid)) Option(dataset.getCoverImage) match { case None => Response.ok(Map("url" -> null)).build() case Some(coverImage) => - val owner = getOwner(ctx, did) - val fullPath = - logicalPath(resourceType, owner.getEmail, dataset.getName, coverImage) - - val document = DocumentFactory - .openReadonlyDocument(FileResolver.resolve(fullPath)) - .asInstanceOf[OnVersionedFileResource] - - val presignedUrl = withLakeFSErrorHandling( - s"generating a presigned URL for cover image '$coverImage'" - ) { - LakeFSStorageClient.getFilePresignedUrl( - document.getRepositoryName(), - document.getVersionHash(), - document.getFileRelativePath() - ) - } - - Response.ok(Map("url" -> presignedUrl)).build() + val url = CoverImageUtils + .openCover(resourceType, getOwner(ctx, did).getEmail, dataset.getName, coverImage) + .map(CoverImageUtils.presignedUrl(_, coverImage)) + Response.ok(Map("url" -> url.orNull)).build() } } } diff --git a/file-service/src/main/scala/org/apache/texera/service/resource/ModelResource.scala b/file-service/src/main/scala/org/apache/texera/service/resource/ModelResource.scala index 7c323928c8..1ef4fddfc7 100644 --- a/file-service/src/main/scala/org/apache/texera/service/resource/ModelResource.scala +++ b/file-service/src/main/scala/org/apache/texera/service/resource/ModelResource.scala @@ -40,11 +40,14 @@ import org.apache.texera.service.`type`.{Diff, ExistingUploadFilesRequest, LakeF import org.apache.texera.service.resource.ResourceTables.{Model => MODEL_RESOURCE} import org.apache.texera.service.resource.ModelAccessResource._ import org.apache.texera.service.resource.ModelResource.{context, _} +import org.apache.texera.service.util.CoverImageUtils +import org.apache.texera.service.util.CoverImageUtils.CoverImageRequest import org.apache.texera.service.util.S3StorageClient import org.apache.texera.service.util.LakeFSExceptionHandler.withLakeFSErrorHandling import org.jooq.{DSLContext, EnumType} import java.io.InputStream +import java.net.URI import java.util.Optional import scala.jdk.CollectionConverters._ import scala.jdk.OptionConverters._ @@ -55,13 +58,22 @@ object ModelResource { // Callers may omit the framework; it is a display label, not a validation gate for files. val DEFAULT_FRAMEWORK = "pytorch" - // Recognised values for the `framework` and `format` labels. They are metadata, not file - // checks -- a loader dispatches on them, so an unknown value is rejected at creation - // rather than surfacing later as an unloadable model. - val SUPPORTED_FRAMEWORKS: Set[String] = Set("pytorch", "tensorflow", "onnx", "sklearn") + // Metadata, not file checks: a loader dispatches on these, so an unknown value is + // rejected up front. "other" is in both sets so an unnamed one never blocks an upload. + val SUPPORTED_FRAMEWORKS: Set[String] = + Set("pytorch", "tensorflow", "onnx", "sklearn", "other") val SUPPORTED_FORMATS: Set[String] = - Set("torchscript", "state-dict", "safetensors", "onnx", "savedmodel", "joblib", "pickle") + Set( + "torchscript", + "state-dict", + "safetensors", + "onnx", + "savedmodel", + "joblib", + "pickle", + "other" + ) private def validateLabel(field: String, value: String, allowed: Set[String]): Unit = { if (!allowed.contains(value)) { @@ -71,6 +83,20 @@ object ModelResource { } } + /** + * Trims a framework/format label, treats blank as absent, validates the rest. + * Shared by create and update so the edit form cannot reject what create accepted. + */ + private def normalizeLabel( + field: String, + value: String, + allowed: Set[String] + ): Option[String] = { + val normalized = Option(value).map(_.trim).filter(_.nonEmpty) + normalized.foreach(validateLabel(field, _, allowed)) + normalized + } + // Matches model_version.name VARCHAR(128). private val MAX_VERSION_NAME_LENGTH = 128 @@ -147,6 +173,10 @@ object ModelResource { case class ModelNameModification(mid: Integer, name: String) + case class ModelFrameworkModification(mid: Integer, framework: String) + + case class ModelFormatModification(mid: Integer, format: String) + case class DashboardModelVersion( modelVersion: ModelVersion, fileNodes: List[LakeFSFileNode] @@ -173,13 +203,7 @@ class ModelResource extends LazyLogging { mid: Integer, requesterUid: Option[Integer] ): DashboardModel = { - val targetModel = getModelByID(ctx, mid) - - if (requesterUid.isEmpty && !targetModel.getIsPublic) { - throw new ForbiddenException(ERR_USER_HAS_NO_ACCESS_TO_MODEL_MESSAGE) - } else if (requesterUid.exists(uid => !userHasReadAccess(ctx, mid, uid))) { - throw new ForbiddenException(ERR_USER_HAS_NO_ACCESS_TO_MODEL_MESSAGE) - } + val targetModel = requireReadAccess(ctx, mid, requesterUid) val userAccessPrivilege = requesterUid .map(uid => getModelUserAccessPrivilege(ctx, mid, uid)) @@ -226,11 +250,9 @@ class ModelResource extends LazyLogging { model.setIsPublic(isModelPublic) model.setIsDownloadable(isModelDownloadable) model.setOwnerUid(uid) - val framework = - Option(request.framework).map(_.trim).filter(_.nonEmpty).getOrElse(DEFAULT_FRAMEWORK) - validateLabel("framework", framework, SUPPORTED_FRAMEWORKS) - val format = Option(request.format).map(_.trim).filter(_.nonEmpty) - format.foreach(validateLabel("format", _, SUPPORTED_FORMATS)) + val framework = normalizeLabel("framework", request.framework, SUPPORTED_FRAMEWORKS) + .getOrElse(DEFAULT_FRAMEWORK) + val format = normalizeLabel("format", request.format, SUPPORTED_FORMATS) model.setFramework(framework) model.setFormat(format.orNull) @@ -339,6 +361,53 @@ class ModelResource extends LazyLogging { } } + @POST + @Consumes(Array(MediaType.APPLICATION_JSON)) + @Produces(Array(MediaType.APPLICATION_JSON)) + @RolesAllowed(Array("REGULAR", "ADMIN")) + @Path("/update/framework") + def updateModelFramework( + modificator: ModelFrameworkModification, + @Auth sessionUser: SessionUser + ): Response = { + withTransaction(context) { ctx => + val modelDao = new ModelDao(ctx.configuration()) + val model = getModelByID(ctx, modificator.mid) + if (!userHasWriteAccess(ctx, modificator.mid, sessionUser.getUid)) { + throw new ForbiddenException(ERR_USER_HAS_NO_ACCESS_TO_MODEL_MESSAGE) + } + + model.setFramework( + normalizeLabel("framework", modificator.framework, SUPPORTED_FRAMEWORKS) + .getOrElse(DEFAULT_FRAMEWORK) + ) + modelDao.update(model) + Response.ok().build() + } + } + + @POST + @Consumes(Array(MediaType.APPLICATION_JSON)) + @Produces(Array(MediaType.APPLICATION_JSON)) + @RolesAllowed(Array("REGULAR", "ADMIN")) + @Path("/update/format") + def updateModelFormat( + modificator: ModelFormatModification, + @Auth sessionUser: SessionUser + ): Response = { + withTransaction(context) { ctx => + val modelDao = new ModelDao(ctx.configuration()) + val model = getModelByID(ctx, modificator.mid) + if (!userHasWriteAccess(ctx, modificator.mid, sessionUser.getUid)) { + throw new ForbiddenException(ERR_USER_HAS_NO_ACCESS_TO_MODEL_MESSAGE) + } + + model.setFormat(normalizeLabel("format", modificator.format, SUPPORTED_FORMATS).orNull) + modelDao.update(model) + Response.ok().build() + } + } + @POST @Consumes(Array(MediaType.APPLICATION_JSON)) @Produces(Array(MediaType.APPLICATION_JSON)) @@ -776,6 +845,22 @@ class ModelResource extends LazyLogging { }) } + /** Version list of a public model, for a logged-out visitor on a hub model page. */ + @GET + @PermitAll + @Path("/{mid}/publicVersion/list") + def getPublicModelVersionList( + @PathParam("mid") mid: Integer + ): List[ModelVersion] = { + withTransaction(context)(ctx => { + val model = getModelByID(ctx, mid) + if (!isModelPublic(ctx, model.getMid)) { + throw new ForbiddenException(ERR_USER_HAS_NO_ACCESS_TO_MODEL_MESSAGE) + } + fetchModelVersions(ctx, model.getMid) + }) + } + @GET @RolesAllowed(Array("REGULAR", "ADMIN")) @Path("/{mid}/version/latest") @@ -807,6 +892,17 @@ class ModelResource extends LazyLogging { withTransaction(context)(ctx => fetchModelVersionRootFileNodes(ctx, mid, mvid, Some(uid))) } + /** Anonymous half of the endpoint above. */ + @GET + @PermitAll + @Path("/{mid}/publicVersion/{mvid}/rootFileNodes") + def retrievePublicModelVersionRootFileNodes( + @PathParam("mid") mid: Integer, + @PathParam("mvid") mvid: Integer + ): ModelVersionRootFileNodesResponse = { + withTransaction(context)(ctx => fetchModelVersionRootFileNodes(ctx, mid, mvid, None)) + } + // =========================================================================== // File upload (one-shot + session-based multipart) // =========================================================================== @@ -911,10 +1007,124 @@ class ModelResource extends LazyLogging { ) } + // =========================================================================== + // Cover image + // =========================================================================== + + /** Points the model card at a committed image inside the model, "<version>/<file>". */ + @POST + @RolesAllowed(Array("REGULAR", "ADMIN")) + @Path("/{mid}/update/cover") + @Consumes(Array(MediaType.APPLICATION_JSON)) + def updateModelCoverImage( + @PathParam("mid") mid: Integer, + request: CoverImageRequest, + @Auth sessionUser: SessionUser + ): Response = { + withTransaction(context) { ctx => + val model = getModelByID(ctx, mid) + if (!userHasWriteAccess(ctx, mid, sessionUser.getUid)) { + throw new ForbiddenException(ERR_USER_HAS_NO_ACCESS_TO_MODEL_MESSAGE) + } + + val normalized = + CoverImageUtils.validatePathOrThrow(request.coverImage, CoverImageUtils.MAX_PATH_LENGTH) + + val document = CoverImageUtils.openCoverOrBadRequest( + ResourceType.Model, + getOwner(ctx, mid).getEmail, + model.getName, + normalized + ) + CoverImageUtils.requireWithinSizeLimit(CoverImageUtils.fileSizeOf(document, normalized)) + + model.setCoverImage(normalized) + new ModelDao(ctx.configuration()).update(model) + Response.ok(Map("coverImage" -> normalized)).build() + } + } + + /** 307 redirect to the cover's presigned S3 URL. */ + @GET + @PermitAll + @Path("/{mid}/cover") + def getModelCover( + @PathParam("mid") mid: Integer, + @Auth sessionUser: Optional[SessionUser] + ): Response = { + withTransaction(context) { ctx => + val model = requireCoverReadAccess(ctx, mid, sessionUser) + val coverImage = Option(model.getCoverImage).getOrElse( + throw new NotFoundException("No cover image") + ) + + val document = CoverImageUtils + .openCover(ResourceType.Model, getOwner(ctx, mid).getEmail, model.getName, coverImage) + .getOrElse(throw new NotFoundException("No cover image")) + + Response + .temporaryRedirect(new URI(CoverImageUtils.presignedUrl(document, coverImage))) + .build() + } + } + + /** + * Presigned cover URL as JSON. Needed for private models because `<img src>` + * cannot attach the Authorization header that GET /{mid}/cover requires. + */ + @GET + @PermitAll + @Path("/{mid}/cover-url") + @Produces(Array(MediaType.APPLICATION_JSON)) + def getModelCoverUrl( + @PathParam("mid") mid: Integer, + @Auth sessionUser: Optional[SessionUser] + ): Response = { + withTransaction(context) { ctx => + val model = requireCoverReadAccess(ctx, mid, sessionUser) + + Option(model.getCoverImage) match { + case None => Response.ok(Map("url" -> null)).build() + case Some(coverImage) => + val url = CoverImageUtils + .openCover(ResourceType.Model, getOwner(ctx, mid).getEmail, model.getName, coverImage) + .map(CoverImageUtils.presignedUrl(_, coverImage)) + Response.ok(Map("url" -> url.orNull)).build() + } + } + } + // =========================================================================== // Private helpers // =========================================================================== + /** A cover is readable by anyone for a public model, and by read-grantees otherwise. */ + private def requireCoverReadAccess( + ctx: DSLContext, + mid: Integer, + sessionUser: Optional[SessionUser] + ): Model = + requireReadAccess(ctx, mid, sessionUser.toScala.map(_.getUid)) + + /** + * The single read rule for a model: anonymous callers get public models only, a + * signed-in caller goes through userHasReadAccess. Shared with getDashboardModel + * so the cover endpoints cannot drift from it. + */ + private def requireReadAccess( + ctx: DSLContext, + mid: Integer, + requesterUid: Option[Integer] + ): Model = { + val model = getModelByID(ctx, mid) + if (requesterUid.isEmpty && !model.getIsPublic) { + throw new ForbiddenException(ERR_USER_HAS_NO_ACCESS_TO_MODEL_MESSAGE) + } else if (requesterUid.exists(uid => !userHasReadAccess(ctx, mid, uid))) { + throw new ForbiddenException(ERR_USER_HAS_NO_ACCESS_TO_MODEL_MESSAGE) + } + model + } + private def fetchModelVersions(ctx: DSLContext, mid: Integer): List[ModelVersion] = { ctx .selectFrom(MODEL_VERSION) diff --git a/file-service/src/main/scala/org/apache/texera/service/util/CoverImageUtils.scala b/file-service/src/main/scala/org/apache/texera/service/util/CoverImageUtils.scala new file mode 100644 index 0000000000..8ac29173c3 --- /dev/null +++ b/file-service/src/main/scala/org/apache/texera/service/util/CoverImageUtils.scala @@ -0,0 +1,134 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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. + */ + +package org.apache.texera.service.util + +import jakarta.ws.rs.BadRequestException +import org.apache.commons.io.FilenameUtils +import org.apache.commons.vfs2.FileNotFoundException +import org.apache.texera.amber.core.storage.model.OnVersionedFileResource +import org.apache.texera.amber.core.storage.util.LakeFSStorageClient +import org.apache.texera.amber.core.storage.{DocumentFactory, FileResolver, ResourceType} +import org.apache.texera.service.resource.ResourceNaming +import org.apache.texera.service.util.LakeFSExceptionHandler.withLakeFSErrorHandling + +/** + * Resource-agnostic halves of the cover-image endpoints, shared by datasets and models. + * The extension allowlist is a security control: a cover is served to the browser as a + * presigned URL, so a duplicated allowlist that drifts is a real risk. + */ +object CoverImageUtils { + + /** Committed image, relative to the resource root, e.g. "v1 - init/cover.jpg". */ + case class CoverImageRequest(coverImage: String) + + val SIZE_LIMIT_BYTES: Long = 10 * 1024 * 1024 // 10 MB + + /** varchar(255) on `model`; `dataset` passes its own narrower limit. */ + val MAX_PATH_LENGTH: Int = 255 + + private val ALLOWED_EXTENSIONS: Set[String] = Set(".jpg", ".jpeg", ".png", ".gif", ".webp") + + /** Normalizes a cover path relative to the resource root and enforces the image allowlist. */ + def validatePathOrThrow(coverImage: String, maxPathLength: Int): String = { + if (coverImage == null || coverImage.trim.isEmpty) { + throw new BadRequestException("Cover image path is required") + } + + val normalized = ResourceNaming.validateAndNormalizeFilePathOrThrow(coverImage) + + val extension = FilenameUtils.getExtension(normalized) + if (extension == null || !ALLOWED_EXTENSIONS.contains(s".$extension".toLowerCase)) { + throw new BadRequestException("Invalid file type") + } + + // FileResolver needs <version>/<file>; a bare name builds a path it cannot parse. + if (normalized.split("/").length < 2) { + throw new BadRequestException( + "Cover image path must be relative to the resource root, as '<version>/<file>'" + ) + } + + // Guard the column width here so an over-long path is a 400, not a jOOQ-wrapped 500. + if (normalized.length > maxPathLength) { + throw new BadRequestException(s"Cover image path must be at most $maxPathLength characters") + } + normalized + } + + /** + * Opens the committed image at the cover path, or None if it no longer resolves. + * An Option because FileResolver's IOException has no mapper and would otherwise be + * an opaque 500 on every card render; callers decide what an unresolvable cover means. + */ + def openCover( + resourceType: ResourceType.Value, + ownerEmail: String, + resourceName: String, + normalized: String + ): Option[OnVersionedFileResource] = + try { + Some( + DocumentFactory + .openReadonlyDocument( + FileResolver.resolve(s"$resourceType/$ownerEmail/$resourceName/$normalized") + ) + .asInstanceOf[OnVersionedFileResource] + ) + } catch { + case _: FileNotFoundException => None + } + + /** Write-path reading of an unresolvable cover: the client sent a bad path. */ + def openCoverOrBadRequest( + resourceType: ResourceType.Value, + ownerEmail: String, + resourceName: String, + normalized: String + ): OnVersionedFileResource = + openCover(resourceType, ownerEmail, resourceName, normalized).getOrElse( + throw new BadRequestException(s"No committed file at cover image path '$normalized'") + ) + + def requireWithinSizeLimit(fileSize: Long): Unit = + if (fileSize > SIZE_LIMIT_BYTES) { + throw new BadRequestException( + s"Cover image must be less than ${SIZE_LIMIT_BYTES / (1024 * 1024)} MB" + ) + } + + def fileSizeOf(document: OnVersionedFileResource, normalized: String): Long = + withLakeFSErrorHandling(s"reading the size of cover image '$normalized'") { + LakeFSStorageClient.getFileSize( + document.getRepositoryName(), + document.getVersionHash(), + document.getFileRelativePath() + ) + } + + /** Presigned S3 URL for an already-authorized cover image. */ + def presignedUrl(document: OnVersionedFileResource, normalized: String): String = + withLakeFSErrorHandling(s"generating a presigned URL for cover image '$normalized'") { + LakeFSStorageClient.getFilePresignedUrl( + document.getRepositoryName(), + document.getVersionHash(), + document.getFileRelativePath() + ) + } +} diff --git a/file-service/src/test/scala/org/apache/texera/service/resource/DatasetResourceSpec.scala b/file-service/src/test/scala/org/apache/texera/service/resource/DatasetResourceSpec.scala index fd1a525331..e0931570fc 100644 --- a/file-service/src/test/scala/org/apache/texera/service/resource/DatasetResourceSpec.scala +++ b/file-service/src/test/scala/org/apache/texera/service/resource/DatasetResourceSpec.scala @@ -44,6 +44,7 @@ import org.apache.texera.dao.jooq.generated.tables.pojos.{ } import org.apache.texera.service.MockLakeFS import org.apache.texera.service.`type`.{ExistingUploadFile, ExistingUploadFilesRequest} +import org.apache.texera.service.util.CoverImageUtils import org.apache.texera.service.util.S3StorageClient import org.jooq.SQLDialect import org.jooq.impl.DSL @@ -3405,7 +3406,7 @@ class DatasetResourceSpec ) maliciousPaths.foreach { path => - val request = DatasetResource.CoverImageRequest(path) + val request = CoverImageUtils.CoverImageRequest(path) assertThrows[BadRequestException] { datasetResource.updateDatasetCoverImage( @@ -3424,7 +3425,7 @@ class DatasetResourceSpec ) absolutePaths.foreach { path => - val request = DatasetResource.CoverImageRequest(path) + val request = CoverImageUtils.CoverImageRequest(path) assertThrows[BadRequestException] { datasetResource.updateDatasetCoverImage( @@ -3444,7 +3445,7 @@ class DatasetResourceSpec ) invalidPaths.foreach { path => - val request = DatasetResource.CoverImageRequest(path) + val request = CoverImageUtils.CoverImageRequest(path) assertThrows[BadRequestException] { datasetResource.updateDatasetCoverImage( @@ -3460,7 +3461,7 @@ class DatasetResourceSpec assertThrows[BadRequestException] { datasetResource.updateDatasetCoverImage( baseDataset.getDid, - DatasetResource.CoverImageRequest(""), + CoverImageUtils.CoverImageRequest(""), sessionUser ) } @@ -3468,14 +3469,14 @@ class DatasetResourceSpec assertThrows[BadRequestException] { datasetResource.updateDatasetCoverImage( baseDataset.getDid, - DatasetResource.CoverImageRequest(null), + CoverImageUtils.CoverImageRequest(null), sessionUser ) } } it should "reject when user lacks WRITE access" in { - val request = DatasetResource.CoverImageRequest("v1/cover.jpg") + val request = CoverImageUtils.CoverImageRequest("v1/cover.jpg") assertThrows[ForbiddenException] { datasetResource.updateDatasetCoverImage( @@ -3489,7 +3490,7 @@ class DatasetResourceSpec it should "set cover image successfully" in { testDatasetVersion - val request = DatasetResource.CoverImageRequest(testCoverImagePath) + val request = CoverImageUtils.CoverImageRequest(testCoverImagePath) val response = datasetResource.updateDatasetCoverImage( baseDataset.getDid, request, diff --git a/file-service/src/test/scala/org/apache/texera/service/resource/ModelApiForUiSpec.scala b/file-service/src/test/scala/org/apache/texera/service/resource/ModelApiForUiSpec.scala index cf55ca09fa..13742b0014 100644 --- a/file-service/src/test/scala/org/apache/texera/service/resource/ModelApiForUiSpec.scala +++ b/file-service/src/test/scala/org/apache/texera/service/resource/ModelApiForUiSpec.scala @@ -350,6 +350,130 @@ class ModelApiForUiSpec newModel(format = null).model.getFormat shouldBe null } + it should "accept \"other\" for a framework or format we have no name for" in { + newModel(framework = "other", format = "other").model.getFramework shouldEqual "other" + } + + "updateModelFramework" should "relabel an existing model" in { + val mid = newModel(framework = "pytorch").model.getMid + + modelResource + .updateModelFramework(ModelResource.ModelFrameworkModification(mid, "onnx"), sessionUser) + .getStatus shouldEqual 200 + + modelResource.getModel(mid, sessionUser).model.getFramework shouldEqual "onnx" + } + + it should "reject a framework outside the supported set" in { + val mid = newModel().model.getMid + + val ex = intercept[BadRequestException] { + modelResource.updateModelFramework( + ModelResource.ModelFrameworkModification(mid, "caffe"), + sessionUser + ) + } + ex.getMessage should include("Unsupported framework 'caffe'") + modelResource.getModel(mid, sessionUser).model.getFramework shouldEqual "pytorch" + } + + it should "reject a caller without write access" in { + val mid = newModel(isPublic = true).model.getMid + + assertThrows[ForbiddenException] { + modelResource.updateModelFramework( + ModelResource.ModelFrameworkModification(mid, "onnx"), + strangerSession + ) + } + } + + it should "reject an unknown model id" in { + assertThrows[NotFoundException] { + modelResource.updateModelFramework( + ModelResource.ModelFrameworkModification(999999, "onnx"), + sessionUser + ) + } + } + + "updateModelFormat" should "set a format on a model created without one" in { + val mid = newModel(format = null).model.getMid + + modelResource + .updateModelFormat(ModelResource.ModelFormatModification(mid, "safetensors"), sessionUser) + .getStatus shouldEqual 200 + + modelResource.getModel(mid, sessionUser).model.getFormat shouldEqual "safetensors" + } + + it should "reject a format outside the supported set" in { + val mid = newModel(format = "pickle").model.getMid + + assertThrows[BadRequestException] { + modelResource.updateModelFormat( + ModelResource.ModelFormatModification(mid, "gguf"), + sessionUser + ) + } + modelResource.getModel(mid, sessionUser).model.getFormat shouldEqual "pickle" + } + + it should "reject a caller without write access" in { + val mid = newModel(isPublic = true).model.getMid + + assertThrows[ForbiddenException] { + modelResource.updateModelFormat( + ModelResource.ModelFormatModification(mid, "onnx"), + strangerSession + ) + } + } + + "the label endpoints" should "trim a framework the way createModel does" in { + val mid = newModel(framework = "pytorch").model.getMid + + modelResource + .updateModelFramework(ModelResource.ModelFrameworkModification(mid, "onnx "), sessionUser) + .getStatus shouldEqual 200 + + modelResource.getModel(mid, sessionUser).model.getFramework shouldEqual "onnx" + } + + it should "treat a blank framework as a reset to the default, not as invalid" in { + val mid = newModel(framework = "onnx").model.getMid + + Seq(null, "", " ").foreach { blank => + modelResource + .updateModelFramework(ModelResource.ModelFrameworkModification(mid, blank), sessionUser) + .getStatus shouldEqual 200 + modelResource.getModel(mid, sessionUser).model.getFramework shouldEqual + ModelResource.DEFAULT_FRAMEWORK + } + } + + // A mislabelled model shouldn't need recreating, so a format must be clearable. + it should "clear the format when given a blank value" in { + val mid = newModel(format = "pickle").model.getMid + + Seq(null, "", " ").foreach { blank => + modelResource + .updateModelFormat(ModelResource.ModelFormatModification(mid, blank), sessionUser) + .getStatus shouldEqual 200 + modelResource.getModel(mid, sessionUser).model.getFormat shouldBe null + } + } + + it should "trim a format the way createModel does" in { + val mid = newModel(format = null).model.getMid + + modelResource + .updateModelFormat(ModelResource.ModelFormatModification(mid, " safetensors "), sessionUser) + .getStatus shouldEqual 200 + + modelResource.getModel(mid, sessionUser).model.getFormat shouldEqual "safetensors" + } + // =========================================================================== // listing sizes // =========================================================================== diff --git a/file-service/src/test/scala/org/apache/texera/service/resource/ModelHubApiSpec.scala b/file-service/src/test/scala/org/apache/texera/service/resource/ModelHubApiSpec.scala new file mode 100644 index 0000000000..8e999f045f --- /dev/null +++ b/file-service/src/test/scala/org/apache/texera/service/resource/ModelHubApiSpec.scala @@ -0,0 +1,398 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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. + */ + +package org.apache.texera.service.resource + +import jakarta.ws.rs._ +import jakarta.ws.rs.core.Response +import org.apache.texera.auth.SessionUser +import org.apache.texera.dao.MockTexeraDB +import org.apache.texera.dao.jooq.generated.enums.{PrivilegeEnum, UserRoleEnum} +import org.apache.texera.dao.jooq.generated.tables.daos.{ModelDao, ModelUserAccessDao, UserDao} +import org.apache.texera.dao.jooq.generated.tables.pojos.{ModelUserAccess, ModelVersion, User} +import org.apache.texera.service.MockLakeFS +import org.apache.texera.service.util.CoverImageUtils +import org.scalatest.flatspec.AnyFlatSpec +import org.scalatest.matchers.should.Matchers +import org.scalatest.{BeforeAndAfterAll, OptionValues} + +import java.io.ByteArrayInputStream +import java.util.Optional + +// What a model page needs beyond the owner's dashboard: the cover image a card +// renders, and the anonymous halves of version browsing. +class ModelHubApiSpec + extends AnyFlatSpec + with Matchers + with OptionValues + with MockTexeraDB + with MockLakeFS + with BeforeAndAfterAll + with ResourceTestHelpers { + + private def mkUser(name: String): User = { + val user = new User + user.setName(name) + user.setEmail(s"[email protected]") + user.setRole(UserRoleEnum.ADMIN) + user + } + + private val ownerUser: User = mkUser("model_hub_owner") + private val readerUser: User = mkUser("model_hub_reader") + private val strangerUser: User = mkUser("model_hub_stranger") + + lazy val modelResource = new ModelResource() + lazy val modelDao = new ModelDao(getDSLContext.configuration()) + lazy val ownerSession = new SessionUser(ownerUser) + lazy val readerSession = new SessionUser(readerUser) + lazy val strangerSession = new SessionUser(strangerUser) + + override protected def beforeAll(): Unit = { + super.beforeAll() + initializeDBAndReplaceDSLContext() + val userDao = new UserDao(getDSLContext.configuration()) + Seq(ownerUser, readerUser, strangerUser).foreach(userDao.insert) + } + + override protected def afterAll(): Unit = { + try shutdownDB() + finally super.afterAll() + } + + // ---------- helpers ---------- + private def newModel(isPublic: Boolean): ModelResource.DashboardModel = + modelResource.createModel( + ModelResource.CreateModelRequest( + modelName = uniqueName("hub-model"), + modelDescription = "for model hub api tests", + isModelPublic = isPublic, + isModelDownloadable = true, + framework = "pytorch", + format = null + ), + ownerSession + ) + + private def upload(mid: Integer, path: String, bytes: Array[Byte]): Unit = + modelResource + .uploadOneFileToModel( + mid, + urlEnc(path), + "upload", + new ByteArrayInputStream(bytes), + mkHeaders(bytes.length.toLong), + ownerSession + ) + .getStatus shouldEqual 200 + + /** Commits `cover.jpg` plus a weights file, returning the generated version. */ + private def commitVersionWithCover(mid: Integer): ModelVersion = { + upload(mid, "cover.jpg", Array.fill[Byte](512)(0xff.toByte)) + upload(mid, "model.pt", Array.fill[Byte](64)(0x1)) + modelResource.createModelVersion("init", mid, ownerSession).modelVersion + } + + private def grantRead(mid: Integer, uid: Integer): Unit = + new ModelUserAccessDao(getDSLContext.configuration()) + .insert(new ModelUserAccess(mid, uid, PrivilegeEnum.READ)) + + private def urlOf(response: Response): Option[String] = + Option( + response.getEntity.asInstanceOf[Map[String, String]]("url") + ) + + private def setPublic(mid: Integer, isPublic: Boolean): Unit = { + val model = modelDao.fetchOneByMid(mid) + model.setIsPublic(isPublic) + modelDao.update(model) + } + + // =========================================================================== + // cover image: setting it + // =========================================================================== + "updateModelCoverImage" should "store the normalized path and echo it back" in { + val mid = newModel(isPublic = false).model.getMid + val version = commitVersionWithCover(mid) + val coverPath = s"${version.getName}/cover.jpg" + + val response = + modelResource.updateModelCoverImage( + mid, + CoverImageUtils.CoverImageRequest(coverPath), + ownerSession + ) + + response.getStatus shouldEqual 200 + response.getEntity.asInstanceOf[Map[String, String]]("coverImage") shouldEqual coverPath + modelDao.fetchOneByMid(mid).getCoverImage shouldEqual coverPath + } + + it should "reject a non-image extension" in { + val mid = newModel(isPublic = false).model.getMid + val version = commitVersionWithCover(mid) + + assertThrows[BadRequestException] { + modelResource.updateModelCoverImage( + mid, + CoverImageUtils.CoverImageRequest(s"${version.getName}/model.pt"), + ownerSession + ) + } + } + + it should "reject an empty path" in { + val mid = newModel(isPublic = false).model.getMid + + assertThrows[BadRequestException] { + modelResource.updateModelCoverImage(mid, CoverImageUtils.CoverImageRequest(""), ownerSession) + } + } + + it should "reject a bare file name, which builds a path FileResolver cannot parse" in { + val mid = newModel(isPublic = false).model.getMid + commitVersionWithCover(mid) + + assertThrows[BadRequestException] { + modelResource.updateModelCoverImage( + mid, + CoverImageUtils.CoverImageRequest("cover.jpg"), + ownerSession + ) + } + } + + it should "reject a well-formed path pointing at a version that does not exist" in { + val mid = newModel(isPublic = false).model.getMid + commitVersionWithCover(mid) + + assertThrows[BadRequestException] { + modelResource.updateModelCoverImage( + mid, + CoverImageUtils.CoverImageRequest("v9 - nope/cover.jpg"), + ownerSession + ) + } + } + + it should "reject a caller with only read access" in { + val mid = newModel(isPublic = false).model.getMid + val version = commitVersionWithCover(mid) + grantRead(mid, readerUser.getUid) + + assertThrows[ForbiddenException] { + modelResource.updateModelCoverImage( + mid, + CoverImageUtils.CoverImageRequest(s"${version.getName}/cover.jpg"), + readerSession + ) + } + } + + // =========================================================================== + // cover image: reading it + // =========================================================================== + "getModelCover" should "redirect a logged-out visitor to the presigned URL of a public model" in { + val mid = newModel(isPublic = true).model.getMid + val version = commitVersionWithCover(mid) + modelResource.updateModelCoverImage( + mid, + CoverImageUtils.CoverImageRequest(s"${version.getName}/cover.jpg"), + ownerSession + ) + + val response = modelResource.getModelCover(mid, Optional.empty()) + + response.getStatus shouldEqual 307 + response.getHeaderString("Location") should not be null + } + + it should "return 404 when no cover image is set" in { + val mid = newModel(isPublic = true).model.getMid + + assertThrows[NotFoundException] { + modelResource.getModelCover(mid, Optional.of(ownerSession)) + } + } + + it should "forbid a logged-out visitor on a private model" in { + val mid = newModel(isPublic = false).model.getMid + + assertThrows[ForbiddenException] { + modelResource.getModelCover(mid, Optional.empty()) + } + } + + it should "forbid a signed-in user with no grant on a private model" in { + val mid = newModel(isPublic = false).model.getMid + + assertThrows[ForbiddenException] { + modelResource.getModelCover(mid, Optional.of(strangerSession)) + } + } + + "getModelCoverUrl" should "return the presigned URL to the owner of a private model" in { + val mid = newModel(isPublic = false).model.getMid + val version = commitVersionWithCover(mid) + modelResource.updateModelCoverImage( + mid, + CoverImageUtils.CoverImageRequest(s"${version.getName}/cover.jpg"), + ownerSession + ) + + val response = modelResource.getModelCoverUrl(mid, Optional.of(ownerSession)) + + response.getStatus shouldEqual 200 + urlOf(response) shouldBe defined + } + + it should "return the presigned URL to a read-grantee of a private model" in { + val mid = newModel(isPublic = false).model.getMid + val version = commitVersionWithCover(mid) + modelResource.updateModelCoverImage( + mid, + CoverImageUtils.CoverImageRequest(s"${version.getName}/cover.jpg"), + ownerSession + ) + grantRead(mid, readerUser.getUid) + + urlOf(modelResource.getModelCoverUrl(mid, Optional.of(readerSession))) shouldBe defined + } + + // Used to be an unmapped IOException: a 500 on every card render, not one bad request. + it should "report no cover, not fail, when the stored path no longer resolves" in { + val mid = newModel(isPublic = true).model.getMid + val version = commitVersionWithCover(mid) + modelResource.updateModelCoverImage( + mid, + CoverImageUtils.CoverImageRequest(s"${version.getName}/cover.jpg"), + ownerSession + ) + + // Point the stored cover at a version that never existed. + val model = modelDao.fetchOneByMid(mid) + model.setCoverImage("v9 - deleted/cover.jpg") + modelDao.update(model) + + val response = modelResource.getModelCoverUrl(mid, Optional.of(ownerSession)) + response.getStatus shouldEqual 200 + urlOf(response) shouldBe empty + + assertThrows[NotFoundException] { + modelResource.getModelCover(mid, Optional.of(ownerSession)) + } + } + + it should "return a null url rather than 404 when no cover image is set" in { + val mid = newModel(isPublic = true).model.getMid + + val response = modelResource.getModelCoverUrl(mid, Optional.of(ownerSession)) + + response.getStatus shouldEqual 200 + urlOf(response) shouldBe empty + } + + it should "forbid a signed-in user with no grant on a private model" in { + val mid = newModel(isPublic = false).model.getMid + + assertThrows[ForbiddenException] { + modelResource.getModelCoverUrl(mid, Optional.of(strangerSession)) + } + } + + // =========================================================================== + // anonymous version browsing + // =========================================================================== + "getPublicModelVersionList" should "list versions of a public model without authentication" in { + val mid = newModel(isPublic = true).model.getMid + val version = commitVersionWithCover(mid) + + modelResource.getPublicModelVersionList(mid).map(_.getMvid) shouldEqual List(version.getMvid) + } + + it should "return an empty list for a public model with no versions" in { + modelResource.getPublicModelVersionList(newModel(isPublic = true).model.getMid) shouldBe empty + } + + it should "forbid listing versions of a private model" in { + val mid = newModel(isPublic = false).model.getMid + + assertThrows[ForbiddenException] { + modelResource.getPublicModelVersionList(mid) + } + } + + it should "reject an unknown model id" in { + assertThrows[NotFoundException] { + modelResource.getPublicModelVersionList(999999) + } + } + + it should "stop listing once the model is unpublished" in { + val mid = newModel(isPublic = true).model.getMid + commitVersionWithCover(mid) + modelResource.getPublicModelVersionList(mid) should have size 1 + + setPublic(mid, isPublic = false) + + assertThrows[ForbiddenException] { + modelResource.getPublicModelVersionList(mid) + } + } + + "retrievePublicModelVersionRootFileNodes" should "return the file tree of a public model version" in { + val mid = newModel(isPublic = true).model.getMid + val version = commitVersionWithCover(mid) + + val response = modelResource.retrievePublicModelVersionRootFileNodes(mid, version.getMvid) + + response.fileNodes.map(_.getName) should contain allOf ("cover.jpg", "model.pt") + response.size shouldEqual 512 + 64 + } + + it should "forbid the file tree of a private model version" in { + val mid = newModel(isPublic = false).model.getMid + val version = commitVersionWithCover(mid) + + assertThrows[ForbiddenException] { + modelResource.retrievePublicModelVersionRootFileNodes(mid, version.getMvid) + } + } + + it should "reject an unknown version id on a public model" in { + val mid = newModel(isPublic = true).model.getMid + + assertThrows[NotFoundException] { + modelResource.retrievePublicModelVersionRootFileNodes(mid, 999999) + } + } + + it should "agree with the authenticated endpoint on the same version" in { + val mid = newModel(isPublic = true).model.getMid + val version = commitVersionWithCover(mid) + + val anonymous = modelResource.retrievePublicModelVersionRootFileNodes(mid, version.getMvid) + val authenticated = + modelResource.retrieveModelVersionRootFileNodes(mid, version.getMvid, ownerSession) + + anonymous.fileNodes.map(_.getName).sorted shouldEqual + authenticated.fileNodes.map(_.getName).sorted + anonymous.size shouldEqual authenticated.size + } +} diff --git a/file-service/src/test/scala/org/apache/texera/service/resource/ModelResourcePermissionsSpec.scala b/file-service/src/test/scala/org/apache/texera/service/resource/ModelResourcePermissionsSpec.scala index 01851aa64b..b225c076d7 100644 --- a/file-service/src/test/scala/org/apache/texera/service/resource/ModelResourcePermissionsSpec.scala +++ b/file-service/src/test/scala/org/apache/texera/service/resource/ModelResourcePermissionsSpec.scala @@ -43,12 +43,16 @@ class ModelResourcePermissionsSpec extends AnyFlatSpec with Matchers { private def rolesOf(m: Method): Option[RolesAllowed] = Option(m.getAnnotation(classOf[RolesAllowed])) - // Anonymous-readable, mirroring the dataset side: the public model read plus the two - // presign routes a public model's files are fetched through. + // Anonymous-readable, mirroring the dataset side. The cover reads do their own + // public/grant check, so a private model's cover is still refused. private val publicEndpointMethods: Set[String] = Set( "getPublicModel", "getPublicPresignedUrl", - "getPublicPresignedUrlWithS3" + "getPublicPresignedUrlWithS3", + "getPublicModelVersionList", + "retrievePublicModelVersionRootFileNodes", + "getModelCover", + "getModelCoverUrl" ) "ModelResource" should "expose HTTP endpoints (sanity check for the reflection scan)" in { diff --git a/file-service/src/test/scala/org/apache/texera/service/util/CoverImageUtilsSpec.scala b/file-service/src/test/scala/org/apache/texera/service/util/CoverImageUtilsSpec.scala new file mode 100644 index 0000000000..bc17afa06b --- /dev/null +++ b/file-service/src/test/scala/org/apache/texera/service/util/CoverImageUtilsSpec.scala @@ -0,0 +1,158 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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. + */ + +package org.apache.texera.service.util + +import jakarta.ws.rs.BadRequestException +import org.scalatest.flatspec.AnyFlatSpec +import org.scalatest.matchers.should.Matchers + +/** + * The extension allowlist is a security control: a cover path is served to the browser + * as a presigned URL, so an active document (.svg, .html) is a stored-XSS vector. + */ +class CoverImageUtilsSpec extends AnyFlatSpec with Matchers { + + private val maxLen = CoverImageUtils.MAX_PATH_LENGTH + + // -- accepted paths --------------------------------------------------------- + + "validatePathOrThrow" should "accept each allowed image extension" in { + Seq("jpg", "jpeg", "png", "gif", "webp").foreach { ext => + CoverImageUtils.validatePathOrThrow(s"v1/cover.$ext", maxLen) shouldBe s"v1/cover.$ext" + } + } + + it should "accept uppercase and mixed-case extensions" in { + CoverImageUtils.validatePathOrThrow("v1/COVER.JPG", maxLen) shouldBe "v1/COVER.JPG" + CoverImageUtils.validatePathOrThrow("v1/Cover.PnG", maxLen) shouldBe "v1/Cover.PnG" + } + + it should "accept the generated version-name form, which contains spaces" in { + CoverImageUtils.validatePathOrThrow("v1 - init/cover.jpg", maxLen) shouldBe + "v1 - init/cover.jpg" + } + + it should "accept unicode in the file name" in { + CoverImageUtils.validatePathOrThrow("v1/カバー画像.jpg", maxLen) shouldBe "v1/カバー画像.jpg" + } + + it should "accept a double extension whose last segment is allowed" in { + CoverImageUtils.validatePathOrThrow("v1/cover.png.jpg", maxLen) shouldBe "v1/cover.png.jpg" + } + + it should "normalize redundant path segments" in { + CoverImageUtils.validatePathOrThrow("v1/./cover.jpg", maxLen) shouldBe "v1/cover.jpg" + CoverImageUtils.validatePathOrThrow("v1/sub/../cover.jpg", maxLen) shouldBe "v1/cover.jpg" + } + + // -- rejected paths --------------------------------------------------------- + + it should "reject path traversal above the root" in { + Seq("../../../etc/passwd", "v1/../../secret.jpg", "../escape.jpg").foreach { p => + a[BadRequestException] should be thrownBy CoverImageUtils.validatePathOrThrow(p, maxLen) + } + } + + it should "reject absolute paths" in { + Seq("/etc/passwd.jpg", "C:/x.jpg").foreach { p => + a[BadRequestException] should be thrownBy CoverImageUtils.validatePathOrThrow(p, maxLen) + } + } + + it should "reject null, empty and whitespace-only paths" in { + Seq(null, "", " ").foreach { p => + a[BadRequestException] should be thrownBy CoverImageUtils.validatePathOrThrow(p, maxLen) + } + } + + it should "reject a non-image extension" in { + Seq("v1/cover.js", "v1/cover.pdf", "v1/cover.csv", "v1/weights.safetensors").foreach { p => + a[BadRequestException] should be thrownBy CoverImageUtils.validatePathOrThrow(p, maxLen) + } + } + + it should "reject .svg and .html, which the browser would execute" in { + Seq("v1/cover.svg", "v1/cover.html", "v1/cover.htm").foreach { p => + a[BadRequestException] should be thrownBy CoverImageUtils.validatePathOrThrow(p, maxLen) + } + } + + it should "reject a path with no extension or a trailing dot" in { + Seq("v1/cover", "v1/cover.").foreach { p => + a[BadRequestException] should be thrownBy CoverImageUtils.validatePathOrThrow(p, maxLen) + } + } + + it should "reject a double extension whose last segment is not allowed" in { + a[BadRequestException] should be thrownBy + CoverImageUtils.validatePathOrThrow("v1/cover.jpg.js", maxLen) + } + + it should "reject a bare file name with no version segment" in { + Seq("cover.jpg", "./cover.png", "sub/../cover.gif").foreach { p => + withClue(s"$p: ") { + a[BadRequestException] should be thrownBy CoverImageUtils.validatePathOrThrow(p, maxLen) + } + } + } + + it should "accept a path nested deeper than <version>/<file>" in { + CoverImageUtils.validatePathOrThrow("v1/img/sub/cover.jpg", maxLen) shouldBe + "v1/img/sub/cover.jpg" + } + + // -- length boundary -------------------------------------------------------- + + it should "accept a path exactly at the column length and reject one over it" in { + val suffix = "/c.jpg" + val atLimit = "v" + "a" * (255 - suffix.length - 1) + suffix + atLimit.length shouldBe 255 + CoverImageUtils.validatePathOrThrow(atLimit, 255) shouldBe atLimit + + val overLimit = atLimit + "x" + a[BadRequestException] should be thrownBy CoverImageUtils.validatePathOrThrow(overLimit, 255) + } + + it should "honour the narrower limit datasets pass" in { + // dataset.cover_image is varchar(246), so the caller's limit must win. + val suffix = "/c.jpg" + val at246 = "v" + "a" * (246 - suffix.length - 1) + suffix + CoverImageUtils.validatePathOrThrow(at246, 246) shouldBe at246 + a[BadRequestException] should be thrownBy CoverImageUtils.validatePathOrThrow(at246, 245) + } + + // -- size limit ------------------------------------------------------------- + + "requireWithinSizeLimit" should "accept a file at exactly the limit" in { + noException should be thrownBy + CoverImageUtils.requireWithinSizeLimit(CoverImageUtils.SIZE_LIMIT_BYTES) + } + + it should "reject a file one byte over the limit, naming the limit in MB" in { + val ex = intercept[BadRequestException] { + CoverImageUtils.requireWithinSizeLimit(CoverImageUtils.SIZE_LIMIT_BYTES + 1) + } + ex.getMessage should include("10 MB") + } + + it should "accept a zero-byte file" in { + noException should be thrownBy CoverImageUtils.requireWithinSizeLimit(0L) + } +} diff --git a/frontend/proxy.config.json b/frontend/proxy.config.json index c26a239b17..bc33371071 100755 --- a/frontend/proxy.config.json +++ b/frontend/proxy.config.json @@ -40,6 +40,16 @@ "secure": false, "changeOrigin": true }, + "/api/model/**": { + "target": "http://localhost:9092";, + "secure": false, + "changeOrigin": true + }, + "/api/access/model/**": { + "target": "http://localhost:9092";, + "secure": false, + "changeOrigin": true + }, "/api/access/computing-unit/**": { "target": "http://localhost:8888";, "secure": false,
