[eugenegujing]

What changes were proposed in this PR?

WorkflowResource.mapWorkflowEntries sets DashboardWorkflow.isOwner by comparing the record's owner uid with the caller's uid. It did so with getUid.eq(uid). Both operands are java.lang.Integer, and Scala's eq is reference identity, so the comparison only succeeded when both boxes were the same object. Two independently autoboxed values are the same object only inside the default java.lang.Integer cache, which spans -128 to 127. This PR changes the comparison to ==, which compares by value and is null-safe on the left operand. The owner uid is now read once per row and feeds both isOwner and ownerId, and the flag is a single expression, uid != null && ownerUid == uid.

Why this is a bug. isOwner answers "is the caller this workflow's owner". For every uid above 127 the answer came back false for the owner's own request, while the ownerId in the same response equalled the caller's uid. The cutoff is a JVM boxing detail, not a property of the data, so the field is correct on a fresh database with small uids and silently wrong once a deployment has more than 127 accounts. WorkflowSearchQueryBuilder.toEntryImpl already computed the same flag with ==, so the two implementations disagreed for the same user and the same workflow. .equals would also compare by value, but baseWorkflowSelect() LEFT JOINs WORKFLOW_OF_USER, so the record's uid can be null and .equals would throw; == is the null-safe form and matches the search builder.

What the flag reaches today. Two endpoints go through mapWorkflowEntries: GET /workflow/list and the hub's getTops, via HubResource.fetchDashboardWorkflowsByWids. No current frontend code reads isOwner from either of them. The dashboard listing that does gate affordances on isOwner is served by GET /dashboard/search, which uses the search builder and was already correct, and the hub landing page renders its own card that does not read the flag. So this PR changes API responses rather than any rendered behavior, and no screenshots apply. It is still a defect in a published REST response and a contradiction between three code paths that answer the same question.

One consequence worth stating plainly. HubResource.getTops has no @Auth parameter and takes uid as a query parameter, so the uid it reflects into isOwner is caller-asserted. Before this change a caller passing someone else's uid only got isOwner: true back for uids inside the Integer cache; afterwards it works for any uid. This widens an existing weakness rather than opening a new one, and it is not privilege escalation: nothing reads that field, and deleteWorkflow independently re-checks ownership against the authenticated session user. isOwner from that endpoint must not be treated as an authorization signal. The endpoint's missing authentication is out of scope here and belongs in its own issue.

The test changes cover both endpoints that go through mapWorkflowEntries.

WorkflowResourceSpec gains one case for the authenticated GET /workflow/list path, set isOwner for an owner whose uid is outside the Integer cache. The suite's existing users have uids 1 and 2, inside the cache, so it could not have caught this. The new case creates an owner with uid 1001, lists through retrieveWorkflowsBySessionUser, and asserts isOwner is true with ownerId 1001 for the owner and false for a user who only holds READ access to the same workflow.

HubResourceSpec, whose uids already sit outside the Integer cache, gains four positive isOwner cases and one merged negative, and drops a stale NOTE comment that documented this bug as deliberately unasserted:

• set isOwner for the owner's own request when the uid is outside the Integer cache (uid 9001) is the regression proper;
• set isOwner for the owner's own request when the uid is inside the Integer cache (uid 42) pins that small uids, which were already correct, stay correct;
• set isOwner for the highest uid inside the Integer cache (127) and set isOwner for the lowest uid outside the Integer cache (128) localise the break to a single increment rather than to the gap between 42 and 9001; and
• clear isOwner for a workflow with no ownership row, for an authenticated and an anonymous caller asserts that a null record uid yields false without throwing, and that both uids being null at once also yields false, where Scala's == reports two nulls as equal and only the uid != null guard keeps the answer false.

The pre-existing negative for a different uid and for an anonymous caller is unchanged.

Any related issues, documentation, discussions?

Closes #8646

Two neighbouring defects were found while working on this and are deliberately not addressed here, since each is a different function and a different failure mode: mapWorkflowEntries dereferences a null privilege for a workflow with no workflow_user_access row, and WorkflowSearchQueryBuilder.toEntryImpl lacks the uid != null guard that this file has, so an anonymous publicSearch caller would report isOwner: true for a workflow with no ownership row.

How was this PR tested?

Unit tests, run against the embedded PostgreSQL that MockTexeraDB provides, so no external database is needed.

sbt "WorkflowExecutionService/testOnly \
  org.apache.texera.web.resource.dashboard.hub.HubResourceSpec \
  org.apache.texera.web.resource.dashboard.file.WorkflowResourceSpec"

sbt "WorkflowExecutionService/testOnly \
  org.apache.texera.web.resource.dashboard.file.WorkflowResourceSpec \
  org.apache.texera.web.resource.dashboard.hub.HubResourceSpec \
  org.apache.texera.web.resource.dashboard.WorkflowSearchQueryBuilderSpec \
  org.apache.texera.web.resource.dashboard.DashboardResourceSpec \
  org.apache.texera.web.resource.dashboard.DatasetSearchQueryBuilderSpec"

sbt scalafmtCheckAll
sbt "scalafixAll --check"

Results:

• HubResourceSpec: 69/69 passed and WorkflowResourceSpec: 81/81 passed;
• the five suites that reach mapWorkflowEntries or assert on an isOwner flag: 190/190 passed;
• scalafmtCheckAll: passed; and
• scalafixAll --check: passed.

Both new behaviours were confirmed to fail without the corresponding source text, not merely to pass with it. Replacing ownerUid == uid with ownerUid.eq(uid) fails exactly three cases, the uid 9001 and uid 128 cases in HubResourceSpec and the uid 1001 case in WorkflowResourceSpec, each with false was not equal to true, while the uid 127 case still passes, which places the break at one increment. Separately, deleting the uid != null guard while keeping == fails exactly one case, the merged no-ownership-row case in HubResourceSpec, on its anonymous-caller assertion with true was not equal to false; no pre-existing test catches that, which is why the case was added.

Was this PR authored or co-authored using generative AI tooling?

Generated-by: Claude Code (Claude Opus 5)

[github-actions]

Automated Reviewer Suggestions

Based on the git blame history of the changed files, we recommend the following reviewers:

• Contributors with relevant context: @yangzhang75, @tanishqgandhi1908
  You can notify them by mentioning @yangzhang75, @tanishqgandhi1908 in a comment.

[github-actions]

Backport auto-label report

This fix: PR was checked against each actively-supported release branch. A release/* label nominates a backport target; the branch's release manager approving this PR is what sends the fix there. The required Backport Approvals check stays red until every label below is approved, so each manager either approves or removes their own label — which is why the labels left on a merged PR are exactly the branches it reached.

Release branch	Analysis
🚫 release/v1.3	Label was removed earlier (opt-out); not re-added. Re-add it by hand if this fix should be backported here after all — the branch's release manager then has to approve before this PR can merge.
🚫 release/v1.2	Label was removed earlier (opt-out); not re-added. Re-add it by hand if this fix should be backported here after all — the branch's release manager then has to approve before this PR can merge.

Auto-label run.

[github-actions]

⚠️ Benchmark changes need a look

🟢 4 better · 🔴 4 worse · ⚪ 7 noise (<±5%) · 0 without baseline

Compared against main 98c6337 benchmarked on this same runner, so the delta is largely free of cross-runner hardware noise. The "7d avg" column still reflects the gh-pages dashboard. Treat <±5% as noise unless repeated.

Dashboard · Run

	config	throughput	MB/s	latency	max Δ latest / 7d
🔴	bs=10 sw=10 sl=64	562	0.343	17,369/22,425/22,425 us	🟢 -28.8% / 🔴 +57.6%
🔴	bs=100 sw=10 sl=64	1,184	0.722	84,653/100,965/100,965 us	🔴 +8.7% / ⚪ within ±5%
⚪	bs=1000 sw=10 sl=64	1,462	0.892	680,144/773,186/773,186 us	⚪ within ±5% / 🟢 +23.7%

Baseline details

Latest main 98c6337 from same runner

config	metric	PR	latest main	7d avg	Δ latest	Δ 7d
bs=10 sw=10 sl=64	throughput	562 tuples/sec	565 tuples/sec	879.16 tuples/sec	-0.5%	-36.1%
bs=10 sw=10 sl=64	MB/s	0.343 MB/s	0.345 MB/s	0.537 MB/s	-0.6%	-36.1%
bs=10 sw=10 sl=64	p50	17,369 us	15,504 us	11,504 us	+12.0%	+51.0%
bs=10 sw=10 sl=64	p95	22,425 us	31,492 us	14,234 us	-28.8%	+57.6%
bs=10 sw=10 sl=64	p99	22,425 us	31,492 us	17,338 us	-28.8%	+29.3%
bs=100 sw=10 sl=64	throughput	1,184 tuples/sec	1,251 tuples/sec	1,147 tuples/sec	-5.4%	+3.2%
bs=100 sw=10 sl=64	MB/s	0.722 MB/s	0.763 MB/s	0.7 MB/s	-5.4%	+3.1%
bs=100 sw=10 sl=64	p50	84,653 us	77,881 us	88,987 us	+8.7%	-4.9%
bs=100 sw=10 sl=64	p95	100,965 us	108,246 us	96,473 us	-6.7%	+4.7%
bs=100 sw=10 sl=64	p99	100,965 us	108,246 us	102,791 us	-6.7%	-1.8%
bs=1000 sw=10 sl=64	throughput	1,462 tuples/sec	1,473 tuples/sec	1,182 tuples/sec	-0.7%	+23.7%
bs=1000 sw=10 sl=64	MB/s	0.892 MB/s	0.899 MB/s	0.721 MB/s	-0.8%	+23.7%
bs=1000 sw=10 sl=64	p50	680,144 us	681,645 us	872,751 us	-0.2%	-22.1%
bs=1000 sw=10 sl=64	p95	773,186 us	747,981 us	910,151 us	+3.4%	-15.0%
bs=1000 sw=10 sl=64	p99	773,186 us	747,981 us	941,479 us	+3.4%	-17.9%

Raw CSV

config_idx,batch_size,schema_width,string_len,num_batches,total_ms,total_tuples,total_bytes,tuples_per_sec,mb_per_sec,lat_p50_us,lat_p95_us,lat_p99_us
0,10,10,64,20,355.86,200,128000,562,0.343,17368.72,22425.14,22425.14
1,100,10,64,20,1689.56,2000,1280000,1184,0.722,84652.56,100965.15,100965.15
2,1000,10,64,20,13679.26,20000,12800000,1462,0.892,680144.17,773185.97,773185.97

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.87%. Comparing base (98c6337) to head (7f06195).
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #8647      +/-   ##
============================================
- Coverage     91.87%   91.87%   -0.01%     
+ Complexity     5049     5048       -1     
============================================
  Files          1265     1265              
  Lines         54915    54915              
  Branches       6940     6940              
============================================
- Hits          50453    50452       -1     
  Misses         2769     2769              
- Partials       1693     1694       +1     

Flag	Coverage Δ		*Carryforward flag
access-control-service	77.38% <ø> (ø)		Carriedforward from 98c6337
agent-service	99.16% <ø> (ø)		Carriedforward from 98c6337
amber	86.06% <100.00%> (-0.01%)	⬇️
computing-unit-managing-service	60.48% <ø> (ø)		Carriedforward from 98c6337
config-service	87.37% <ø> (ø)		Carriedforward from 98c6337
file-service	82.00% <ø> (ø)		Carriedforward from 98c6337
frontend	96.50% <ø> (ø)		Carriedforward from 98c6337
notebook-migration-service	83.73% <ø> (ø)		Carriedforward from 98c6337
pyamber	98.54% <ø> (ø)		Carriedforward from 98c6337
workflow-compiling-service	72.43% <ø> (ø)		Carriedforward from 98c6337

*This pull request uses carry forward flags. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[eugenegujing]

/request-review @aglinxinyuan

[kunwp1]

Looks good!

[Copilot]

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

3 open issues

• These tests mix the returned id from seedWorkflow(...) with hard-coded WID literals when… · New
• These tests mix the returned id from seedWorkflow(...) with hard-coded WID literals when… · New
• These comments state the Integer cache is exactly -128..127 and that 'two boxes of the same value… · New

What changed in this PR

Fixes incorrect isOwner computation for dashboard workflows by switching from reference equality (eq) to value equality (==) when comparing boxed java.lang.Integer UIDs, and adds regression tests that cover both sides of the JVM Integer cache boundary and null-ownership edge cases.

Changes:

• Update WorkflowResource.mapWorkflowEntries to compare owner UID and caller UID by value (==) instead of reference (eq).
• Add HubResourceSpec coverage for isOwner with cached vs uncached Integer values (42, 127, 128, 9001) and for workflows lacking an ownership row (null owner UID).

File	Description
amber/​src/​main/​scala/​org/​apache/​texera/​web/​resource/​dashboard/​user/​workflow/​WorkflowResource.scala	Fixes isOwner calculation by using value equality for boxed Integer UID comparison.
amber/​src/​test/​scala/​org/​apache/​texera/​web/​resource/​dashboard/​hub/​HubResourceSpec.scala	Adds regression tests ensuring isOwner is correctly computed across Integer cache boundary and null owner UID scenarios.

_{Review effort: Lite}