From af2482aab2288c2472fa782f6618c7e5f243b0ff Mon Sep 17 00:00:00 2001 From: kolaente Date: Thu, 4 Jun 2026 23:08:26 +0200 Subject: [PATCH] fix(labels): report owner-level max_permission MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Label writes/deletes are owner-only (CanUpdate/CanDelete), but hasAccessToLabel derived max_permission from the accessible task's permission with a read fallback for the creator branch — so owners showed as read-only and a task-admin reading a label via that task showed as a label admin. Derive it from ownership instead: owner -> admin, otherwise read. Corrects the value CanRead returns for both v1's x-max-permission header and the new v2 max_permission body field. --- pkg/models/label_permissions.go | 35 ++++++--------------------------- pkg/models/label_test.go | 18 ++++++++--------- 2 files changed, 15 insertions(+), 38 deletions(-) diff --git a/pkg/models/label_permissions.go b/pkg/models/label_permissions.go index 1c5df95c1..75468d1bb 100644 --- a/pkg/models/label_permissions.go +++ b/pkg/models/label_permissions.go @@ -107,38 +107,15 @@ func (l *Label) hasAccessToLabel(s *xorm.Session, a web.Auth) (has bool, maxPerm return } - // maxPermission is derived only from label_tasks rows whose task is - // actually accessible. The pre-fix code used Get(ll) against the - // unrestricted LEFT JOIN, so it could return an inaccessible row and - // yield a wrong (or errored) permission. - accessibleTaskIDs := []int64{} - err = s.Table("label_tasks"). - Join("INNER", "tasks", "tasks.id = label_tasks.task_id"). - Where(builder.And( - builder.Eq{"label_tasks.label_id": l.ID}, - accessibleProjects, - )). - Cols("label_tasks.task_id"). - Find(&accessibleTaskIDs) + // Writes and deletes are owner-only (CanUpdate/CanDelete), so the caller's + // max permission is admin for the owner and read for anyone else who can see it. + owner, err := l.isLabelOwner(s, a) if err != nil { return } - - for _, taskID := range accessibleTaskIDs { - t := &Task{ID: taskID} - _, taskPermission, tErr := t.CanRead(s, a) - if tErr != nil { - err = tErr - return - } - if taskPermission > maxPermission { - maxPermission = taskPermission - } - } - - // Creator-branch fallback: access came from created_by_id with no - // accessible task to derive a permission from. - if len(accessibleTaskIDs) == 0 { + if owner { + maxPermission = int(PermissionAdmin) + } else { maxPermission = int(PermissionRead) } diff --git a/pkg/models/label_test.go b/pkg/models/label_test.go index aa73ae647..5320678b3 100644 --- a/pkg/models/label_test.go +++ b/pkg/models/label_test.go @@ -228,7 +228,7 @@ func TestLabel_ReadOne(t *testing.T) { }, auth: &user.User{ID: 1}, assertMaxPermission: true, - wantMaxPermission: int(PermissionRead), + wantMaxPermission: int(PermissionAdmin), }, { name: "Get nonexistant label", @@ -249,8 +249,8 @@ func TestLabel_ReadOne(t *testing.T) { auth: &user.User{ID: 1}, }, { - // Label 4 is attached to tasks in project 1 (user 1 is admin), - // so the accessible-tasks iteration must yield PermissionAdmin. + // Label 4 is owned by user 2; user 1 can read it via a shared task + // but is not the owner, so max permission is read. name: "Get label #4 - other user", fields: fields{ ID: 4, @@ -276,7 +276,7 @@ func TestLabel_ReadOne(t *testing.T) { }, auth: &user.User{ID: 1}, assertMaxPermission: true, - wantMaxPermission: int(PermissionAdmin), + wantMaxPermission: int(PermissionRead), }, { // PoC for GHSA-hj5c-mhh2-g7jq: label 6 is reachable only via task @@ -304,12 +304,12 @@ func TestLabel_ReadOne(t *testing.T) { }, auth: &user.User{ID: 1}, assertMaxPermission: true, - wantMaxPermission: int(PermissionRead), + wantMaxPermission: int(PermissionAdmin), }, { - // Label 8's only label_tasks row points at inaccessible task 34, - // so access must come from the creator branch and the - // maxPermission fallback to PermissionRead must kick in. + // Label 8's only label_tasks row points at inaccessible task 34, so + // access comes from the creator branch; as the owner, user 1's max + // permission is admin. name: "creator can read own label only attached to inaccessible task", fields: fields{ ID: 8, @@ -324,7 +324,7 @@ func TestLabel_ReadOne(t *testing.T) { }, auth: &user.User{ID: 1}, assertMaxPermission: true, - wantMaxPermission: int(PermissionRead), + wantMaxPermission: int(PermissionAdmin), }, { // Non-creator must not be able to read an unattached label owned