Add Adobe Marketo Engage gatekeeper - #394
Conversation
Preview:
|
|
Findings
|
|
Findings
|
|
Findings
|
|
Posted 5 actionable inline findings. |
|
Posted 5 actionable inline findings. |
|
Findings
|
|
Findings
|
|
Posted 9 actionable inline findings. |
|
Findings
|
| if (isEmailDesignerAction(action) && | ||
| (action.type === "designerLifecycle" || action.type === "designerDelete")) { | ||
| for (let dependent of action.affectedDependents) { | ||
| if (dependent.contentType === undefined || dependent.contentType.toLowerCase() === "email") { |
There was a problem hiding this comment.
[P1] Serialize Designer deletes against non-email dependents. This skips every captured dependent whose contentType is not email, even though the used-by response and tests include Smart Campaign dependents. If a campaign deletion/mutation is queued first and this Designer asset deletion second, applying the asset deletion first finds no shared resource key and can remove or fail on an asset while the campaign still references it, reversing the submitted order. Map supported dependent types to their action resource keys or conservatively serialize unknown dependents.
| } | ||
|
|
||
| #businessObjectKeys(action: BusinessObjectAction): string[] { | ||
| let identities = [[BUSINESS_OBJECTS[action.kind].idField], BUSINESS_OBJECTS[action.kind].dedupeFields]; |
There was a problem hiding this comment.
[P1] Preserve ordering across business-object lookup aliases. Keys are derived only from identities present in each submitted payload, but neither matching mode requires the other identity. A dedupe-matched update {externalCompanyId: "acme", name: "first"} and a later ID-matched update {id: 7, name: "second"} can target the same company without sharing a key; approving the latter first leaves first, while submission order leaves second. The cross-strategy test currently supplies both aliases, masking normal valid inputs. Resolve aliases before queuing or serialize same-kind writes conservatively.
| target: Record<string, unknown>, | ||
| snapshot: DesignerDeleteSnapshot, | ||
| ): boolean { | ||
| return JSON.stringify(designerDeleteSnapshot(target)) === JSON.stringify(snapshot); |
There was a problem hiding this comment.
[P2] Exclude request-only settings from delete preflight snapshots. brandedDomain and dedicatedIp are deliberately stripped for create/update postflight because Marketo omits them from EmailSettingsResponseDTO, but an earlier pending update merges them into the delete snapshot and this exact comparison retains them. After update-then-delete is approved in order, the delete preflight can never match the conforming provider read, so the valid delete remains permanently retryable until rejected and resubmitted. Apply the same request-only normalization to delete snapshots/comparison.
| if (page.result.some(record => !matches(record))) { | ||
| throw new MarketoError(`Marketo returned a ${this.kind} record outside the requested filter.`); | ||
| } | ||
| let idField = BUSINESS_OBJECTS[this.kind].idField; |
There was a problem hiding this comment.
[P2] Validate standard-object identities before authorizing records. idField is used only to shape a projection; rows are never required to contain a valid, unique identifier. For example, a successful { externalCompanyId: "acme" } row passes filter correlation and is authorized even though the public contract guarantees identifying fields and the record cannot be reliably correlated or targeted later. Validate positive numeric IDs/non-empty GUIDs and per-page uniqueness, analogous to validateCustomObjectRecords().
|
Posted 4 actionable inline findings. |
There was a problem hiding this comment.
Devin Review found 1 new potential issue.
1 flag not posted on this PR by your GitHub settings — view it in Devin Review. (Configure)
| if ((action.type === "campaignTrigger" || | ||
| action.type === "campaignLifecycle" && action.operation === "activate") && | ||
| (!Number.isSafeInteger(campaign.flowId) || campaign.flowId! <= 0)) { |
There was a problem hiding this comment.
🟡 Invalid campaigns reach scheduling
#preflightCampaignOwnership omits schedule-time type and flow checks. A changed or empty campaign reaches Marketo and the approved schedule fails.
Prompt for agents
In packages/gatekeeper-marketo/src/marketo.ts, extend MarketoGatekeeperImpl.#preflightCampaignOwnership for campaignSchedule actions. Revalidate immediately before dispatch that the target remains a batch campaign and has a valid positive flowId, matching the submission-time checks in MarketoSmartCampaignImpl.schedule. Treat failures as DesignerPreDispatchError so no scheduling request is sent and the approval remains safely retryable.
Was this helpful? React with 👍 or 👎 to provide feedback.
|
Findings
|
|
Superseded by #413, rebuilt from current main with a clean two-commit history and the retained security fixes. |
Summary
Testing
pnpm exec vp run -F @gadgets/marketo-gatekeeper --no-cache buildpnpm exec vp run -F @gadgets/marketo-gatekeeper --no-cache test(239 tests)node --test scripts/release/manifest-lib.test.tspnpm lintpnpm testreaches an unrelated existing Google configurator test failure becauseURLPatternis unavailable in its Node environment.