Skip to content

Commit a42e330

Browse files
committed
fix(cloudflare): stop the rate limiting action defaulting on a replacing update
Making action required on update_rate_limit_rule was only half the fix: the Action dropdown still seeded block for the update operation too, so an update that edited only the threshold kept sending block and converted a live log or challenge rule into a hard block — exactly the harm the required flag was meant to prevent. The update now has its own control with no seeded value, so the action is something the caller states rather than inherits. The certificate status filter also still offered Active and Pending, which Cloudflare does not document for that endpoint; the only documented value is all, and omitting it returns active packs.
1 parent cfacc4d commit a42e330

5 files changed

Lines changed: 49 additions & 13 deletions

File tree

apps/sim/blocks/blocks/cloudflare.ts

Lines changed: 28 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ const SUBBLOCK_ALIASES: Record<string, Record<string, string>> = {
3535
purge_cache: { tags: 'purgeTags' },
3636
create_ruleset: { name: 'rulesetName' },
3737
create_rate_limit_rule: { action: 'rateLimitAction' },
38-
update_rate_limit_rule: { action: 'rateLimitAction' },
38+
update_rate_limit_rule: { action: 'updateRateLimitAction' },
3939
create_access_application: { type: 'appType', tags: 'accessAppTags' },
4040
update_access_application: { type: 'appType', tags: 'accessAppTags' },
4141
list_access_applications: { name: 'listNameFilter', domain: 'accessAppDomainFilter' },
@@ -801,9 +801,8 @@ export const CloudflareBlock: BlockConfig<CloudflareResponse> = {
801801
title: 'Status Filter',
802802
type: 'dropdown',
803803
options: [
804-
{ label: 'All', id: 'all' },
805-
{ label: 'Active', id: 'active' },
806-
{ label: 'Pending', id: 'pending' },
804+
{ label: 'All statuses', id: 'all' },
805+
{ label: 'Active only', id: '' },
807806
],
808807
value: () => 'all',
809808
condition: { field: 'operation', value: 'list_certificates' },
@@ -1346,10 +1345,27 @@ Return ONLY the JSON array - no explanations, no markdown fences.`,
13461345
{ label: 'Log', id: 'log' },
13471346
],
13481347
value: () => 'block',
1349-
condition: {
1350-
field: 'operation',
1351-
value: ['create_rate_limit_rule', 'update_rate_limit_rule'],
1352-
},
1348+
condition: { field: 'operation', value: 'create_rate_limit_rule' },
1349+
},
1350+
{
1351+
/**
1352+
* The update endpoint replaces the rule, so a seeded action would rewrite
1353+
* whatever the rule currently does the moment anything else is edited.
1354+
* This control carries no default: the user has to state the action the
1355+
* replaced rule should end up with.
1356+
*/
1357+
id: 'updateRateLimitAction',
1358+
title: 'Action',
1359+
type: 'dropdown',
1360+
options: [
1361+
{ label: 'Block', id: 'block' },
1362+
{ label: 'Managed Challenge', id: 'managed_challenge' },
1363+
{ label: 'JS Challenge', id: 'js_challenge' },
1364+
{ label: 'Interactive Challenge', id: 'challenge' },
1365+
{ label: 'Log', id: 'log' },
1366+
],
1367+
required: true,
1368+
condition: { field: 'operation', value: 'update_rate_limit_rule' },
13531369
},
13541370
{
13551371
id: 'expression',
@@ -2511,6 +2527,10 @@ Return ONLY the JSON array - no explanations, no markdown fences.`,
25112527
type: 'string',
25122528
description: 'Action applied once a rate limit is exceeded',
25132529
},
2530+
updateRateLimitAction: {
2531+
type: 'string',
2532+
description: 'Action a replaced rate limiting rule ends up applying',
2533+
},
25142534
content: { type: 'string', description: 'DNS record content' },
25152535
ttl: { type: 'number', description: 'Time to live in seconds' },
25162536
proxied: { type: 'boolean', description: 'Whether Cloudflare proxy is enabled' },

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

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,13 +101,28 @@ describe('subBlock ids that share a tool param keep their own default', () => {
101101
expect(mapped.action).toBeUndefined()
102102
})
103103

104-
it('keeps the block default on the rate limiting operations', () => {
104+
it('keeps the block default when creating a rate limiting rule', () => {
105105
expect(mapFor('create_rate_limit_rule', { zoneId: 'zone1', rulesetId: 'rs1' }).action).toBe(
106106
'block'
107107
)
108+
})
109+
110+
it('never seeds an action onto a rate limiting rule it is about to replace', () => {
111+
// The update endpoint replaces the rule, so a seeded block would convert a
112+
// live log or challenge rule into a hard block the moment anything else is
113+
// edited. The user has to state the action instead.
108114
expect(
109115
mapFor('update_rate_limit_rule', { zoneId: 'zone1', rulesetId: 'rs1', ruleId: 'r1' }).action
110-
).toBe('block')
116+
).toBeUndefined()
117+
118+
expect(
119+
mapFor('update_rate_limit_rule', {
120+
zoneId: 'zone1',
121+
rulesetId: 'rs1',
122+
ruleId: 'r1',
123+
updateRateLimitAction: 'log',
124+
}).action
125+
).toBe('log')
111126
})
112127

113128
it('strips the aliased control ids so they never reach a tool as params', () => {
@@ -119,6 +134,7 @@ describe('subBlock ids that share a tool param keep their own default', () => {
119134
'certificateStatus',
120135
'appType',
121136
'rateLimitAction',
137+
'updateRateLimitAction',
122138
'rulesetName',
123139
'zoneNameFilter',
124140
'zoneType',

apps/sim/tools/generated/tool-ids.ts

Lines changed: 1 addition & 1 deletion
Large diffs are not rendered by default.

apps/sim/tools/generated/tool-metadata.ts

Lines changed: 1 addition & 1 deletion
Large diffs are not rendered by default.

apps/sim/tools/generated/tool-outputs.ts

Lines changed: 1 addition & 1 deletion
Large diffs are not rendered by default.

0 commit comments

Comments
 (0)