Skip to content

Commit 81b5a61

Browse files
committed
fix(cloudflare): stop the Access replacements seeding a type and a decision
Same class as the rate limiting action: both Access updates are full replacements, and the shared controls seeded self_hosted and allow for the update operations too. Editing only a policy's include rules would silently convert a live deny, bypass, or non_identity policy to allow — widening who gets in — and editing an application would rewrite what it IS. Each update now has its own required control with no seeded value, so the type and the decision are stated rather than inherited. Regression tests cover both, and the canvas sentence follows the renamed decision control.
1 parent a42e330 commit 81b5a61

2 files changed

Lines changed: 87 additions & 14 deletions

File tree

apps/sim/blocks/blocks/cloudflare.ts

Lines changed: 59 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,8 @@ const SUBBLOCK_ALIASES: Record<string, Record<string, string>> = {
3737
create_rate_limit_rule: { action: 'rateLimitAction' },
3838
update_rate_limit_rule: { action: 'updateRateLimitAction' },
3939
create_access_application: { type: 'appType', tags: 'accessAppTags' },
40-
update_access_application: { type: 'appType', tags: 'accessAppTags' },
40+
update_access_application: { type: 'updateAppType', tags: 'accessAppTags' },
41+
update_access_policy: { decision: 'updatePolicyDecision' },
4142
list_access_applications: { name: 'listNameFilter', domain: 'accessAppDomainFilter' },
4243
list_access_groups: { name: 'listNameFilter' },
4344
list_access_service_tokens: { name: 'listNameFilter' },
@@ -184,7 +185,7 @@ export const CloudflareBlock: BlockConfig<CloudflareResponse> = {
184185
],
185186
update_access_policy: [
186187
{ text: 'Update Access policy', field: 'policyId', core: true },
187-
{ text: 'to', field: 'decision' },
188+
{ text: 'to', field: 'updatePolicyDecision' },
188189
{ text: 'on application', field: 'appId' },
189190
],
190191
delete_access_policy: [
@@ -1696,10 +1697,35 @@ Return ONLY the comma-separated list - no explanations, no extra text.`,
16961697
],
16971698
value: () => 'self_hosted',
16981699
required: true,
1699-
condition: {
1700-
field: 'operation',
1701-
value: ['create_access_application', 'update_access_application'],
1702-
},
1700+
condition: { field: 'operation', value: 'create_access_application' },
1701+
},
1702+
{
1703+
/**
1704+
* Updating an Access application replaces it, so a seeded type would
1705+
* rewrite what a live application IS as soon as anything else is edited.
1706+
* No default: the caller states the type the replaced application keeps.
1707+
*/
1708+
id: 'updateAppType',
1709+
title: 'Application Type',
1710+
type: 'dropdown',
1711+
options: [
1712+
{ label: 'Self-Hosted', id: 'self_hosted' },
1713+
{ label: 'SaaS', id: 'saas' },
1714+
{ label: 'SSH', id: 'ssh' },
1715+
{ label: 'VNC', id: 'vnc' },
1716+
{ label: 'App Launcher', id: 'app_launcher' },
1717+
{ label: 'WARP', id: 'warp' },
1718+
{ label: 'Browser Isolation', id: 'biso' },
1719+
{ label: 'Bookmark', id: 'bookmark' },
1720+
{ label: 'Dashboard SSO', id: 'dash_sso' },
1721+
{ label: 'Infrastructure', id: 'infrastructure' },
1722+
{ label: 'RDP', id: 'rdp' },
1723+
{ label: 'MCP', id: 'mcp' },
1724+
{ label: 'MCP Portal', id: 'mcp_portal' },
1725+
{ label: 'Proxy Endpoint', id: 'proxy_endpoint' },
1726+
],
1727+
required: true,
1728+
condition: { field: 'operation', value: 'update_access_application' },
17031729
},
17041730
{
17051731
id: 'domain',
@@ -1927,10 +1953,25 @@ Return ONLY the comma-separated list - no explanations, no extra text.`,
19271953
],
19281954
value: () => 'allow',
19291955
required: true,
1930-
condition: {
1931-
field: 'operation',
1932-
value: ['create_access_policy', 'update_access_policy'],
1933-
},
1956+
condition: { field: 'operation', value: 'create_access_policy' },
1957+
},
1958+
{
1959+
/**
1960+
* Updating a policy replaces it, so a seeded allow would silently widen a
1961+
* live deny, bypass, or non_identity policy the moment its rules are
1962+
* edited. No default: the caller states the decision.
1963+
*/
1964+
id: 'updatePolicyDecision',
1965+
title: 'Decision',
1966+
type: 'dropdown',
1967+
options: [
1968+
{ label: 'Allow', id: 'allow' },
1969+
{ label: 'Deny', id: 'deny' },
1970+
{ label: 'Non-Identity (service tokens)', id: 'non_identity' },
1971+
{ label: 'Bypass (skip Access entirely)', id: 'bypass' },
1972+
],
1973+
required: true,
1974+
condition: { field: 'operation', value: 'update_access_policy' },
19341975
},
19351976
{
19361977
id: 'include',
@@ -2523,6 +2564,14 @@ Return ONLY the JSON array - no explanations, no markdown fences.`,
25232564
tunnelStatus: { type: 'string', description: 'Status filter when listing tunnels' },
25242565

25252566
appType: { type: 'string', description: 'Access application type' },
2567+
updateAppType: {
2568+
type: 'string',
2569+
description: 'Access application type a replaced application ends up with',
2570+
},
2571+
updatePolicyDecision: {
2572+
type: 'string',
2573+
description: 'Decision a replaced Access policy ends up applying',
2574+
},
25262575
rateLimitAction: {
25272576
type: 'string',
25282577
description: 'Action applied once a rate limit is exceeded',

apps/sim/tools/cloudflare/cloudflare.test.ts

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,11 +58,33 @@ describe('subBlock ids that share a tool param keep their own default', () => {
5858
expect(mapped.type).not.toBe('self_hosted')
5959
})
6060

61-
it('keeps the Access application type on the Access operations', () => {
61+
it('keeps the Access application type when creating an application', () => {
6262
expect(mapFor('create_access_application', { accountId: 'acct1' }).type).toBe('self_hosted')
63-
expect(mapFor('update_access_application', { accountId: 'acct1', appId: 'app1' }).type).toBe(
64-
'self_hosted'
65-
)
63+
})
64+
65+
it('never seeds a type or decision onto an Access resource it is about to replace', () => {
66+
// Both Access updates are full replacements, so a seeded value rewrites what
67+
// the live resource IS as soon as anything else is edited — a policy would
68+
// flip from deny to allow, an application from saas to self_hosted.
69+
const app = mapFor('update_access_application', { accountId: 'acct1', appId: 'app1' })
70+
expect(app.type).toBeUndefined()
71+
expect(
72+
mapFor('update_access_application', {
73+
accountId: 'acct1',
74+
appId: 'app1',
75+
updateAppType: 'saas',
76+
}).type
77+
).toBe('saas')
78+
79+
const policy = mapFor('update_access_policy', { accountId: 'acct1', policyId: 'p1' })
80+
expect(policy.decision).toBeUndefined()
81+
expect(
82+
mapFor('update_access_policy', {
83+
accountId: 'acct1',
84+
policyId: 'p1',
85+
updatePolicyDecision: 'deny',
86+
}).decision
87+
).toBe('deny')
6688
})
6789

6890
it('leaves the DNS record type unset on the filter operations', () => {
@@ -133,6 +155,8 @@ describe('subBlock ids that share a tool param keep their own default', () => {
133155
'recordProxied',
134156
'certificateStatus',
135157
'appType',
158+
'updateAppType',
159+
'updatePolicyDecision',
136160
'rateLimitAction',
137161
'updateRateLimitAction',
138162
'rulesetName',

0 commit comments

Comments
 (0)