Skip to content

Commit 160aaaa

Browse files
committed
fix(gitlab): validate list_groups min-access-level and similarity ordering at execution time
Greptile round 1 (both P1): - min_access_level: coerce via the shared access-level enum (coerceGitLabMinAccessLevel) and throw on out-of-enum values (31, 999) or 0, so a direct tool call fails loudly instead of sending an invalid filter to GitLab - order_by=similarity now requires a non-empty search term (GitLab ignores similarity ordering without one); throws a clear error otherwise - tests for both validations
1 parent 6237de1 commit 160aaaa

4 files changed

Lines changed: 93 additions & 5 deletions

File tree

apps/sim/tools/gitlab/groups.test.ts

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,40 @@ describe('gitlab_list_groups', () => {
9494
)
9595
})
9696

97+
it('forwards the documented visibility, min-access-level, and all-available filters', () => {
98+
const url = gitlabListGroupsTool.request.url({
99+
accessToken: 'pat',
100+
visibility: 'private',
101+
minAccessLevel: 30,
102+
allAvailable: true,
103+
})
104+
expect(url).toBe(
105+
'https://gitlab.com/api/v4/groups?visibility=private&min_access_level=30&all_available=true'
106+
)
107+
})
108+
109+
it('rejects an out-of-enum minimum access level instead of sending it to GitLab', () => {
110+
expect(() =>
111+
gitlabListGroupsTool.request.url({ accessToken: 'pat', minAccessLevel: 31 })
112+
).toThrow(/Invalid GitLab access level/)
113+
expect(() =>
114+
gitlabListGroupsTool.request.url({ accessToken: 'pat', minAccessLevel: 0 })
115+
).toThrow(/Invalid GitLab access level/)
116+
})
117+
118+
it('rejects similarity ordering without a search term, but allows it with one', () => {
119+
expect(() =>
120+
gitlabListGroupsTool.request.url({ accessToken: 'pat', orderBy: 'similarity' })
121+
).toThrow(/similarity/)
122+
expect(
123+
gitlabListGroupsTool.request.url({
124+
accessToken: 'pat',
125+
orderBy: 'similarity',
126+
search: 'plat',
127+
})
128+
).toBe('https://gitlab.com/api/v4/groups?search=plat&order_by=similarity')
129+
})
130+
97131
it('reads the total from the x-total header, falling back to length', async () => {
98132
const withHeader = await gitlabListGroupsTool.transformResponse?.(
99133
mockResponse({ json: [{ id: 1 }, { id: 2 }], headers: { 'x-total': '17' } }),

apps/sim/tools/gitlab/list_groups.ts

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import type { GitLabListGroupsParams, GitLabListGroupsResponse } from '@/tools/gitlab/types'
2-
import { getGitLabApiBase } from '@/tools/gitlab/utils'
2+
import { coerceGitLabMinAccessLevel, getGitLabApiBase } from '@/tools/gitlab/utils'
33
import type { ToolConfig } from '@/tools/types'
44

55
export const gitlabListGroupsTool: ToolConfig<GitLabListGroupsParams, GitLabListGroupsResponse> = {
@@ -89,15 +89,24 @@ export const gitlabListGroupsTool: ToolConfig<GitLabListGroupsParams, GitLabList
8989
request: {
9090
url: (params) => {
9191
const queryParams = new URLSearchParams()
92+
const search = params.search?.trim()
9293
if (params.owned) queryParams.append('owned', 'true')
93-
if (params.search) queryParams.append('search', params.search)
94+
if (search) queryParams.append('search', search)
9495
if (params.topLevelOnly) queryParams.append('top_level_only', 'true')
9596
if (params.visibility) queryParams.append('visibility', params.visibility)
96-
if (params.minAccessLevel && params.minAccessLevel > 0) {
97-
queryParams.append('min_access_level', String(params.minAccessLevel))
97+
const minAccessLevel = coerceGitLabMinAccessLevel(params.minAccessLevel)
98+
if (minAccessLevel !== undefined) {
99+
queryParams.append('min_access_level', String(minAccessLevel))
98100
}
99101
if (params.allAvailable) queryParams.append('all_available', 'true')
100-
if (params.orderBy) queryParams.append('order_by', params.orderBy)
102+
if (params.orderBy) {
103+
if (params.orderBy === 'similarity' && !search) {
104+
throw new Error(
105+
'GitLab list groups: order_by "similarity" requires a non-empty search term.'
106+
)
107+
}
108+
queryParams.append('order_by', params.orderBy)
109+
}
101110
if (params.sort) queryParams.append('sort', params.sort)
102111
if (params.perPage) queryParams.append('per_page', String(params.perPage))
103112
if (params.page) queryParams.append('page', String(params.page))

apps/sim/tools/gitlab/utils.test.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import { describe, expect, it } from 'vitest'
55
import {
66
coerceGitLabAccessLevel,
7+
coerceGitLabMinAccessLevel,
78
getGitLabApiBase,
89
getGitLabResourcePath,
910
hasGitLabAccessLevel,
@@ -139,6 +140,33 @@ describe('coerceGitLabAccessLevel', () => {
139140
})
140141
})
141142

143+
describe('coerceGitLabMinAccessLevel', () => {
144+
it('returns undefined for absent or blank values', () => {
145+
expect(coerceGitLabMinAccessLevel(undefined)).toBeUndefined()
146+
expect(coerceGitLabMinAccessLevel(null)).toBeUndefined()
147+
expect(coerceGitLabMinAccessLevel('')).toBeUndefined()
148+
})
149+
150+
it('coerces valid filter levels from integer, numeric string, or name', () => {
151+
expect(coerceGitLabMinAccessLevel(30)).toBe(30)
152+
expect(coerceGitLabMinAccessLevel('30')).toBe(30)
153+
expect(coerceGitLabMinAccessLevel('Developer')).toBe(30)
154+
expect(coerceGitLabMinAccessLevel(5)).toBe(5)
155+
})
156+
157+
it('rejects zero ("No access") — GitLab\'s filter floor is 5', () => {
158+
expect(() => coerceGitLabMinAccessLevel(0)).toThrow(InvalidGitLabAccessLevelError)
159+
expect(() => coerceGitLabMinAccessLevel('No access')).toThrow(InvalidGitLabAccessLevelError)
160+
})
161+
162+
it('rejects out-of-enum values so they never reach GitLab', () => {
163+
expect(() => coerceGitLabMinAccessLevel(31)).toThrow(InvalidGitLabAccessLevelError)
164+
expect(() => coerceGitLabMinAccessLevel(999)).toThrow(InvalidGitLabAccessLevelError)
165+
expect(() => coerceGitLabMinAccessLevel('35')).toThrow(InvalidGitLabAccessLevelError)
166+
expect(() => coerceGitLabMinAccessLevel('root')).toThrow(InvalidGitLabAccessLevelError)
167+
})
168+
})
169+
142170
describe('hasGitLabAccessLevel', () => {
143171
it('treats numeric zero ("No access") and its string form as provided', () => {
144172
expect(hasGitLabAccessLevel(0)).toBe(true)

apps/sim/tools/gitlab/utils.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,23 @@ export function coerceGitLabAccessLevel(value: unknown): number {
149149
throw new InvalidGitLabAccessLevelError(value)
150150
}
151151

152+
/**
153+
* Coerces an optional GitLab `min_access_level` filter value. Returns undefined
154+
* when the field is absent or blank, otherwise coerces via the shared
155+
* access-level rules and rejects "No access" (0) — GitLab's minimum-access
156+
* filter floor is 5 (Minimal access). Runs at execution time so a direct tool
157+
* call with an out-of-enum value (e.g. 31 or 999) fails loudly with a clear
158+
* error instead of sending an invalid filter to GitLab.
159+
*
160+
* @throws {InvalidGitLabAccessLevelError} when the value is not a valid filter level.
161+
*/
162+
export function coerceGitLabMinAccessLevel(value: unknown): number | undefined {
163+
if (value === undefined || value === null || value === '') return undefined
164+
const level = coerceGitLabAccessLevel(value)
165+
if (level === 0) throw new InvalidGitLabAccessLevelError(value)
166+
return level
167+
}
168+
152169
/**
153170
* A GitLab access/membership resource is scoped either to a project or a group.
154171
* The two share an identical endpoint surface (`/members`, `/invitations`,

0 commit comments

Comments
 (0)