fix(integrations): close regressions found in the final validation sweep - #6764
Conversation
An independent read-only audit of the eight integrations merged to staging today found defects in every one, most of them side effects of the surgery those PRs performed on already-shipped code. Data loss and destructive paths: - cloudflare: restore the shipped subBlock ids on read filters so existing workflows keep their DNS/zone/purge filters. Losing them made list_dns_records return the entire zone with success: true, which a downstream delete fan-out would then target. The colliding write controls are renamed instead, chosen by blast radius. - cloudflare: refuse an update_ruleset_rule that would tear down the rule it edits. PATCH is a replace, so an omitted action_parameters unbound the WAF managed ruleset and every override under it. - cloudflare: split the hidden `enabled` control so a value set while drafting can no longer disable a live WAF or rate-limiting rule. - cloudflare: stop `name` leaking into update_dns_record and renaming a live record. - okta: stop a blank name overwriting a stored group name via the LLM path. The block guard covered only the UI. Broken on the default path: - microsoft_ad: update_user sent accountEnabled: "" on its own default, so every call left at "No Change" failed. Same tri-state defect already fixed for forceChangePasswordNextSignInWithMfa; `visibility` fixed alongside it. - cloudflare: `domain` is required for self_hosted (the default app type), ssh, vnc and rdp; add saas_app/target_criteria and drop dash_sso, which has no request variant. Silent wrong results: - datadog: list_monitors inherited Create Monitor's tag filter and returned a filtered list as if complete. - servicenow: `fields` carried both a JSON body and a projection on the three legacy generic operations. The regression test for this fed already-JSON and could not fail; it now feeds a real projection. - splunk: cancel_search_job reported failure on success by parsing an XML body as JSON; readSplunkJson now tolerates it. - okta: sendEmail === true dropped a string 'true', silently skipping the deactivation email. Security: - mssql: add writetext/updatetext/readtext to the statement screen. \bupdate\b cannot match UPDATETEXT, so both were reachable through the read-only path. - crowdstrike: chunk repeated-query ids. At the published caps a single request built a ~68 KB query string, past typical proxy limits. Also: splunk count=0 unbounded read, splunk pagination totals, the `nobody` placeholder that reintroduced the namespace bug by copy-paste, okta cursor and activate controls split per operation, servicenow sysparm_having syntax and two required controls no longer pre-seeded with consequential values, datadog block outputs reconciled with tool outputs, and 16 escaped apostrophes that corrupted the published Entra docs. One scope removed from microsoft_ad (User.Read.All). Directory.Read.All and GroupMember.ReadWrite.All were proposed for removal and verified still required; a test now asserts they stay.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryHigh Risk Overview Cloudflare restores shipped subBlock ids for read filters ( CrowdStrike batches by-id GET/DELETE calls under a ~4 KB URL budget with partial-delete error reporting; MSSQL read-only validation adds Datadog / Entra / Okta / ServiceNow / Splunk fix param mappers (leaked filters, tri-state Reviewed by Cursor Bugbot for commit 4395da3. Configure here. |
Greptile SummaryThe PR performs a broad regression sweep across eight integrations, restoring persisted block-state compatibility and correcting request mapping, response parsing, pagination, destructive-update safeguards, and generated metadata.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains, and the previously reported Splunk and CrowdStrike issues are fixed or invalidated by the current code and thread acknowledgments.
|
| Filename | Overview |
|---|---|
| apps/sim/tools/splunk/utils.ts | The dispatch reader now extracts SIDs only from complete Splunk response envelopes while strict JSON parsing remains in place for search results. |
| apps/sim/app/api/tools/crowdstrike/query/operations.ts | By-ID requests are URL-budgeted and destructive batch failures preserve confirmed committed IDs without overstating partial successes. |
| apps/sim/blocks/blocks/cloudflare.ts | Cloudflare operation controls and parameter mappings are split to prevent unrelated stored values from entering destructive update requests. |
| apps/sim/blocks/blocks/servicenow.ts | ServiceNow body fields and response projections are separated by operation to avoid conflating request payloads with query parameters. |
| apps/sim/lib/workflows/migrations/subblock-migrations.ts | Saved workflow sub-block identifiers are migrated for renamed integration controls to preserve persisted configuration. |
| apps/sim/app/api/tools/mssql/utils.ts | Read-only SQL validation now recognizes additional mutating statement families that previously bypassed keyword matching. |
Reviews (5): Last reviewed commit: "fix(splunk): read the dispatch XML envel..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
…ike deletes Narrows readSplunkJson's non-JSON tolerance to XML. The dispatching and job-control endpoints answer in XML, but a body that is neither empty nor XML was meant to be JSON, so swallowing its parse failure handed get_search_results an empty envelope and reported a lost result set as a search with zero events. Annotates a batched CrowdStrike delete that fails partway with the IDs its earlier batches already removed. Falcon cannot roll those back, so a bare failure left the caller unable to tell what was gone and a blind retry re-targeted IDs that no longer existed. Registers the Cloudflare subblock-ID migration the registry-stability check requires. The suffixed read-filter IDs never shipped in a release and every block already materializes the restored IDs, so they are dropped rather than renamed onto values the collision guard would discard anyway.
|
Pushed 906b23f addressing both Greptile P1s, plus the Splunk — malformed responses became empty results (P1). CrowdStrike — chunk failure hid prior deletions (P1). A batched CI — Cloudflare subblock-ID stability. The check flagged seven IDs removed by the read-filter restore. They are registered as Each of the three fixes has a regression test that was individually verified to fail when its fix is reverted. |
An execute rule sent an explicit empty action_parameters object past the new
guard, because presence was checked rather than emptiness. `{}` is the same
payload Cloudflare's schema default produces, so it unbound the managed ruleset
the guard exists to protect.
Datadog's new List Monitors pagination used a bare `Number()`, so a typo or an
unresolved reference in either advanced field reached Datadog as a literal NaN
— the pattern this same sweep fixed for Entra `top` and the Splunk numerics.
Okta's block still marked the group name required on update, blocking a
description-only update that the tool, its merge helper, and the API all accept.
|
@cursor review |
…ke commit list Splitting the XML tolerance out of readSplunkJson into readSplunkDispatchJson puts it only on the three dispatching and job-control tools that need it. The results path can no longer read any non-JSON body as an empty envelope, so a 2xx HTML interstitial surfaces instead of reporting a search that matched nothing. The dispatch reader anchors on the one documented `<response>` root, so an interstitial fails there too. A batched delete now records the IDs Falcon echoed in `resources` rather than the IDs that were requested. A batch can answer 200 while reporting per-ID failures, and naming those as deleted told the caller to drop still-live indicators from the retry.
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 15de5c2. Configure here.
…d request A 2xx envelope carrying per-ID errors is a partial success, not a failure — failedWithoutResources fails the operation only when nothing came back at all. The batched path already reports it exactly as a single request does, with deletedIds naming what Falcon confirmed and errors naming what it refused. Pin that so the contract is not mistaken for a swallowed failure.
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 0531d3c. Configure here.
A dispatch answering in the documented XML form was replaced with an empty
object, so create_search_job and dispatch_saved_search threw a missing-sid error
after the remote job had already been created — stranding a job the caller could
no longer poll or cancel. The envelope is now projected onto the same `{ sid }`
shape output_mode=json produces, so the search ID survives.
Matching only the opening tag also accepted a body cut off mid-transfer, which
on a cancellation reported a truncated response as a successful cancel. The
pattern now spans the closing tag, so a truncated envelope falls through to
JSON.parse and throws.
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 4395da3. Configure here.
1 participant
An independent read-only audit of the eight integrations merged to
stagingtoday — one cold agent per integration, each researching vendor docs from scratch — found defects in every one. Most are side effects of the surgery those PRs performed on already-shipped code, which is exactly what the sweep was looking for.Every fix has a regression test, and each test was individually verified to fail when its fix is reverted.
Data loss / destructive
list_dns_recordssent no filters and returned the entire zone withsuccess: true— and any downstreamdelete_dns_recordfan-out would then target every record. Fixed by restoring the shipped id on the read side and renaming the write side instead, chosen by blast radius: a lost read filter is destructive, a lost write field just omits from a PATCH.update_ruleset_ruleunbound WAF managed rulesets.PATCHis a replace; an omittedaction_parametersdroppedaction_parameters.idon anexecuterule, detaching the managed ruleset and every override under it. It now refuses rather than silently tearing down the rule it edits.enabledwas one advanced subBlock shared by all four rule operations; advanced controls serialize before theirconditionis evaluated, so a value set while drafting silently disabled a live WAF or rate-limiting rule.namerenamed live DNS records when carried from another operation.Broken on the default path
update_userfailed on every call left at its default.accountEnableddefaults to'', the mapper omitted the key, andfinalInputs = {...inputs, ...transformedParams}left the raw''to reach Graph as a non-boolean. Same tri-state defect already fixed forforceChangePasswordNextSignInWithMfa;visibilityhad it too.domainwas unconditionally optional but is required forself_hosted(the dropdown default),ssh,vnc,rdp. Addedsaas_app/target_criteria; droppeddash_sso, which has no request variant.Silent wrong results
list_monitorsinherited Create Monitor's tag filter, returning a filtered list as if complete.fieldscarried both a JSON body and a projection on the three legacy generic operations. The regression test named for this fed already-JSON and could not fail; it now feeds a real projection.cancel_search_jobreported failure on success, parsing an XML body as JSON.sendEmail === truedropped a string'true', silently skipping the deactivation email on LLM and API-triggered runs.Security
WRITETEXT/UPDATETEXTwere reachable through the read-only path.\bupdate\bcannot matchUPDATETEXT— there is no word boundary afterupdate.idsbuilt ~68 KB query strings at the published caps, past typical proxy limits. Chunked at 4 KB rather than lowering vendor-published caps.Also
Splunk
count=0unbounded read, Splunk pagination totals, thenobodyplaceholder that reintroduced the namespace bug by copy-paste, Okta cursor/activate controls split per operation, ServiceNowsysparm_havingsyntax, two ServiceNow required controls no longer pre-seeded with consequential values (Move Change State defaulted to moving a change backwards; Approve/Reject defaulted to approve), Datadog block outputs reconciled with tool outputs, and 16 escaped apostrophes corrupting the published Entra docs.Decisions worth reviewer attention
User.Read.All).Directory.Read.AllandGroupMember.ReadWrite.Allwere proposed for removal and verified still required —GET /subscribedSkusaccepts neither of our license scopes, andgroup-post-membersdoes not acceptGroup.ReadWrite.All. A test now asserts both stay.0is now sent forautoCancel/maxCount/dispatch*, where it was previously dropped. Correct per Splunk (auto_cancel=0= never cancel) but semantic, not just a NaN guard.certificateStatus,dnsOrder) because reverting would re-open a real leak. Recovering those stored values needs a state migration; nothing at the block level can do it.csrutil/reg/eventlogare real read-tier RTR commands (the swagger prose is stale and there is no enum onbase_command); Cloudflare's Access policy endpoint targeting stays nested, because the reusable endpoint rejectsprecedence; Okta'sactivatedoutput keeps its name to avoid orphaning saved references.Verification
type-checkclean,biomeclean, 41 test files / 820 tests passing,tool-metadata:check,integration-catalog:check,docs:checkandcheck:api-validationall pass. Artifacts regenerated with no unrelated drift.