Missing Permission Check in Team Unit Access

[pedrogaudencio]

Problem

Location: models/perm/access/repo_permission.go (lines 352-354)

teamMode, _ := team.UnitAccessModeEx(ctx, u.Type)
unitAccessMode := max(perm.unitsMode[u.Type], minAccessMode, teamMode)
perm.unitsMode[u.Type] = unitAccessMode

Problem: The original code checked if the team had explicit permissions (exist variable). By removing this check and blindly using teamMode (which could be 0/AccessModeNone), you might be incorrectly setting permissions.

Original code:

unitAccessMode := minAccessMode
if teamMode, exist := team.UnitAccessModeEx(ctx, u.Type); exist {
    unitAccessMode = max(perm.unitsMode[u.Type], unitAccessMode, teamMode)
}

Solution

Fix: Restore the existence check:

for _, team := range teams {
    if teamMode, exist := team.UnitAccessModeEx(ctx, u.Type); exist {
        unitAccessMode := max(perm.unitsMode[u.Type], minAccessMode, teamMode)
        perm.unitsMode[u.Type] = unitAccessMode
    }
}

[pedrogaudencio]

go-gitea/pull/34827 introduced the exist check pattern and go-gitea/pull/35501 intentionally removed the exist check. According to the commit message and PR description:

The logic completely fails in case the team does not have a unit for the specific unit type defined, in which case the logic inserted the minimumVisibility, overriding any previous aggregation of access modes for the unit.
This is resolved by simply always merging the unit access mode of the team as it will simply default to None in case the team does not have a permission defined for the unit, which will be swallowed by the max(..) call in favour of the previous aggregated permission.

Key insight

When UnitAccessModeEx returns (AccessModeNone, false), the max() function will naturally ignore the AccessModeNone value in favour of higher permissions.

teamMode, _ := team.UnitAccessModeEx(ctx, u.Type)
unitAccessMode := max(perm.unitsMode[u.Type], minAccessMode, teamMode)
perm.unitsMode[u.Type] = unitAccessMode

Why it works

1. When a team doesn't have a unit configured, teamMode = AccessModeNone (0)
2. The max() function naturally ignores 0 in favour of higher values
3. This prevents teams without unit permissions from downgrading existing permissions
4. This fixes the bug introduced in commit 6a97ab0 where minAccessMode could override collaborator permissions

Additional Context

Other callers of UnitAccessModeEx:

• models/organization/team.go:175 - UnitAccessMode() wrapper (also ignores exist)
• models/perm/access/repo_permission.go:351 - This code (correct usage)

Both callers intentionally ignore the exist return value because AccessModeNone is the correct default when a unit doesn't exist.

Conclusion

/cc @taoeffect: this is correct, tested, upstream code and intentional design by upstream Gitea.

Evidence:

• Merged into upstream Gitea on September 17, 2025
• Approved by 2 maintainers (@wxiaoguang, @lunny)
• Comprehensive test coverage added
• Fixes a real bug Team permissions on private repos are not merged correctly go-gitea/gitea#35154 where collaborator permissions were incorrectly overridden
• Backported to Gitea 1.24 Correctly override user unitmodes (#35501) go-gitea/gitea#35666