Skip to content

Commit 32b606c

Browse files
committed
fix(github): name the branch-protection parse failure without echoing the input
1 parent 552ebd1 commit 32b606c

2 files changed

Lines changed: 32 additions & 6 deletions

File tree

apps/sim/tools/github/branch_protection_and_workflow_runs.test.ts

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -100,10 +100,23 @@ describe('github_update_branch_protection body builder', () => {
100100
expect(body.restrictions).toEqual(restrictions)
101101
})
102102

103-
it('rejects a non-object JSON value instead of shipping it to GitHub', () => {
103+
it.each(['["octocat"]', '42', 'not json at all', '{"users":'])(
104+
'rejects %p instead of shipping it to GitHub',
105+
(restrictions) => {
106+
expect(() => buildProtectionBody({ ...BASE_PROTECTION_PARAMS, restrictions })).toThrow(
107+
/must be a JSON object/
108+
)
109+
}
110+
)
111+
112+
it('does not echo the rejected value into the error message', () => {
104113
expect(() =>
105-
buildProtectionBody({ ...BASE_PROTECTION_PARAMS, restrictions: '["octocat"]' })
106-
).toThrow(/JSON objects/)
114+
buildProtectionBody({ ...BASE_PROTECTION_PARAMS, restrictions: 'ghp_secretlooking' })
115+
).toThrow(
116+
expect.objectContaining({
117+
message: expect.not.stringContaining('ghp_secretlooking'),
118+
})
119+
)
107120
})
108121

109122
it("coerces the dropdown's 'true'/'false' strings to booleans", () => {

apps/sim/tools/github/update_branch_protection.ts

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,14 @@ import type { BranchProtectionResponse, UpdateBranchProtectionParams } from '@/t
22
import { BRANCH_PROTECTION_OUTPUT_PROPERTIES } from '@/tools/github/types'
33
import type { ToolConfig } from '@/tools/types'
44

5+
/**
6+
* Names the failure class without echoing the rejected text. `JSON.parse` quotes
7+
* the input it rejected back into its own message, and these fields can carry
8+
* values resolved from other blocks.
9+
*/
10+
const BRANCH_PROTECTION_SHAPE_ERROR =
11+
'Branch protection fields must be a JSON object, or left empty to disable the rule'
12+
513
/**
614
* GitHub documents `required_status_checks`, `enforce_admins`,
715
* `required_pull_request_reviews` and `restrictions` as required body fields
@@ -16,15 +24,20 @@ function toNullableObject(value: unknown): Record<string, unknown> | null {
1624
if (typeof value === 'string') {
1725
const trimmed = value.trim()
1826
if (trimmed === '' || trimmed === 'null') return null
19-
const parsed: unknown = JSON.parse(trimmed)
27+
let parsed: unknown
28+
try {
29+
parsed = JSON.parse(trimmed)
30+
} catch {
31+
throw new Error(BRANCH_PROTECTION_SHAPE_ERROR)
32+
}
2033
if (parsed === null) return null
2134
if (typeof parsed !== 'object' || Array.isArray(parsed)) {
22-
throw new Error('Branch protection fields must be JSON objects')
35+
throw new Error(BRANCH_PROTECTION_SHAPE_ERROR)
2336
}
2437
return parsed as Record<string, unknown>
2538
}
2639
if (typeof value !== 'object' || Array.isArray(value)) {
27-
throw new Error('Branch protection fields must be JSON objects')
40+
throw new Error(BRANCH_PROTECTION_SHAPE_ERROR)
2841
}
2942
return value as Record<string, unknown>
3043
}

0 commit comments

Comments
 (0)