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-7479-bbd5c976d7512f796251e5d5a8ca1495e5b3756f in repository https://gitbox.apache.org/repos/asf/texera.git
commit ca41bdd0cd1c99a164868afe6b0dce2d4c502105 Author: Xinyuan Lin <[email protected]> AuthorDate: Sun Aug 9 18:16:23 2026 -0700 test(amber): pin the dataset search's access-control branches (#7479) ### What changes were proposed in this PR? Every member of `DatasetSearchQueryBuilder` is `override protected`, so nothing is directly callable. The one public route in is the trait's `final constructQuery`, and what it returns can be rendered to SQL and inspected without ever executing it. That reaches the part of the file with real consequences: **which datasets a caller is allowed to see.** Adds 9 tests. The one that matters most is the grant join's scoping predicate — without `.eq(uid)` the `UID.isNotNull` check below is satisfied by any user's grant row, which hands the caller every shared dataset in the system. An anonymous caller must see public datasets only and match no grant row at all; a private-only search must not leak public datasets in. Also covers the keyword split, `selectDistinct` being the sole dedup (this builder alone has no `GROUP BY`, so the DISTINCT is all that collapses the rows the access join multiplies out), and the `'dataset'` literal that `DashboardResource` dispatches on with no default branch. **Verified by mutation**, all reverted (production diff empty): | Mutation | Result | |---|---| | grant join not scoped to the caller | red | | anonymous caller matches any grant row | red | | anonymous arm drops the public restriction | red | | private-only search leaks public datasets | red | | `includePublic` arm narrowed to public only | red | | keyword splitting removed | red | | a `GROUP BY` is introduced | red | | resource type mis-tagged | red | `toEntryImpl` is deliberately left uncovered — it is ~80% of this file's uncovered lines and sits behind a live LakeFS call with no mockable seam, so reaching it would need a source change rather than a test. That is stated in the spec's header so the next reader does not re-derive it. `MockTexeraDB` is initialized only because `SearchQueryBuilder.context` reads `SqlServer.getInstance()`; no query is run against the database. No production file is touched. ### Any related issues, documentation, discussions? Closes #7476 ### How was this PR tested? ``` sbt "WorkflowExecutionService/testOnly org.apache.texera.web.resource.dashboard.DatasetSearchQueryBuilderSpec" ``` ``` [info] Tests: succeeded 9, failed 0, canceled 0, ignored 0, pending 0 [info] All tests passed. ``` `Test/scalafmtCheck` and `Test/scalafix --check` both pass. ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (Opus 5) Co-authored-by: Meng Wang <[email protected]> --- .../dashboard/DatasetSearchQueryBuilderSpec.scala | 155 +++++++++++++++++++++ 1 file changed, 155 insertions(+) diff --git a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/DatasetSearchQueryBuilderSpec.scala b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/DatasetSearchQueryBuilderSpec.scala new file mode 100644 index 0000000000..6e760e119c --- /dev/null +++ b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/DatasetSearchQueryBuilderSpec.scala @@ -0,0 +1,155 @@ +/* + * 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.web.resource.dashboard + +import org.apache.texera.dao.MockTexeraDB +import org.apache.texera.web.resource.dashboard.DashboardResource.SearchQueryParams +import org.scalatest.BeforeAndAfterAll +import org.scalatest.flatspec.AnyFlatSpec +import org.scalatest.matchers.should.Matchers + +import scala.jdk.CollectionConverters._ + +/** + * Covers the access-control shape of the SQL `DatasetSearchQueryBuilder` produces. + * + * Every member of the builder is `override protected`, so unlike the workflow and project arms + * nothing here is directly callable. The one public route in is the trait's `final constructQuery`, + * and what it returns can be rendered to SQL and inspected without ever executing it — so these are + * assertions about the query's *shape*, not about rows. + * + * What that buys is the piece of this file with real consequences: which datasets a caller is + * allowed to see. Three branches decide it and none is otherwise tested — + * + * - the join predicate `.and(if (uid == null) DSL.falseCondition() else DATASET_USER_ACCESS.UID.eq(uid))`, + * which scopes the grant rows to the caller. Without the `eq(uid)` the `UID.isNotNull` check + * below matches *anyone's* grant row, which is a complete sharing bypass; + * - the anonymous arm, which must restrict to public datasets and nothing else; + * - `includePublic == false`, which must return only explicitly-granted datasets and must not + * leak public ones in. + * + * `initializeDBAndReplaceDSLContext` is needed only because `SearchQueryBuilder.context` reads + * `SqlServer.getInstance()`; no query is run against the database. + * + * Deliberately not covered: `toEntryImpl` (lines 128-164) is ~80% of this file's uncovered lines + * and sits behind a live LakeFS `retrieveRepositorySize` call with no mockable seam. Reaching it + * would need a source change (injecting a repository-size provider), not a test. + */ +class DatasetSearchQueryBuilderSpec + extends AnyFlatSpec + with Matchers + with BeforeAndAfterAll + with MockTexeraDB { + + private val uid: Integer = Integer.valueOf(42) + + override protected def beforeAll(): Unit = { + initializeDBAndReplaceDSLContext() + } + + override protected def afterAll(): Unit = { + shutdownDB() + } + + private def params(keywords: String*): SearchQueryParams = + SearchQueryParams(keywords = keywords.toList.asJava) + + /** + * The query rendered with its bind values inlined, lower-cased and with the identifier quoting + * stripped, so assertions read as `dataset.is_public` rather than `"texera_db"."dataset"."is_public"`. + */ + private def sqlFor( + callerUid: Integer, + includePublic: Boolean, + p: SearchQueryParams = params() + ): String = + getDSLContext + .renderInlined(DatasetSearchQueryBuilder.constructQuery(callerUid, p, includePublic)) + .toLowerCase + .replace("\"", "") + + "an anonymous search" should "be restricted to public datasets" in { + val sql = sqlFor(null, includePublic = true) + + sql should include("dataset.is_public = true") + sql should not include "dataset_user_access.uid is not null" + } + + it should "match no grant rows at all" in { + // The join is still written, but its predicate is a constant false, so an anonymous caller can + // never pick up somebody else's access row. + val sql = sqlFor(null, includePublic = true) + + sql should include("false") + sql should not include "dataset_user_access.uid = " + } + + it should "ignore includePublic, which only applies to a signed-in caller" in { + sqlFor(null, includePublic = true) shouldBe sqlFor(null, includePublic = false) + } + + "a signed-in search" should "scope the grant join to that caller" in { + // Without this predicate the `uid is not null` test below is satisfied by ANY user's grant row, + // which would hand the caller every shared dataset in the system. + val sql = sqlFor(uid, includePublic = false) + + sql should include(s"dataset_user_access.uid = $uid") + } + + it should "return only explicitly granted datasets when public ones are excluded" in { + val sql = sqlFor(uid, includePublic = false) + + sql should include("dataset_user_access.uid is not null") + sql should not include "is_public = true" + } + + it should "add public datasets to the granted ones when they are included" in { + val sql = sqlFor(uid, includePublic = true) + + sql should include("dataset.is_public = true") + sql should include("dataset_user_access.uid is not null") + sql should include(" or ") + } + + "the keyword filter" should "split on the punctuation the full-text engine reserves" in { + // A user pasting `a+b` means two terms, not a literal. The split set is this file's own and is + // not shared with the other builders. + val sql = sqlFor(uid, includePublic = true, params("alpha+beta")) + + sql should include("alpha") + sql should include("beta") + sql should not include "alpha+beta" + } + + "the query" should "dedupe with selectDistinct rather than a group by" in { + // getGroupByFields is empty here, unlike the workflow and project builders, so the DISTINCT is + // the only thing collapsing the rows the access join multiplies out. + val sql = sqlFor(uid, includePublic = true) + + sql should include("select distinct") + sql should not include "group by" + } + + it should "tag its rows as datasets" in { + // DashboardResource dispatches on this literal with no default branch, so a change here is a + // MatchError at fetch time rather than a compile error. + sqlFor(uid, includePublic = true) should include("'dataset'") + } +}
