fix: destination secret fields #2249
Conversation
WalkthroughThis PR updates many destination db-config.json files to remove or reduce secret/include keys and adds post-schema validation rules ensuring secretKeys are not exposed via includeKeys and that includeKeys are present when device/hybrid modes are supported; schema documentation and tests were updated accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer / CI
participant Validator as validateDestinationDefinitions()
participant Schema as JSON Schema Validator
participant Rules as applyAdditionalRulesValidation()
participant RuleA as Rule: secretKeys ∉ includeKeys
participant RuleB as Rule: includeKeys required for device/hybrid
Dev->>Validator: Provide destination configs
Validator->>Schema: Validate with JSON schema
Schema-->>Validator: Schema result (valid/invalid)
alt Schema valid
Validator->>Rules: Run additional rules
Rules->>RuleA: Check secret vs include/exclude
RuleA-->>Rules: Pass/Fail + details
Rules->>RuleB: Check supportedConnectionModes vs includeKeys
RuleB-->>Rules: Pass/Fail + details
Rules-->>Validator: Aggregated rule results
alt All pass
Validator-->>Dev: Validation success
else One or more fail
Validator-->>Dev: Validation errors (aggregate)
end
else Schema invalid
Schema-->>Dev: Schema validation errors
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #2249 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 2 2
Lines 72 102 +30
Branches 8 21 +13
=========================================
+ Hits 72 102 +30 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
saikumarrs
force-pushed
the
feature/int-4282-review-all-the-integration-definition-configurations-for
branch
from
54c6226 to
ac954b7
Compare
saikumarrs
requested review from
a team,
1abhishekpandey,
ItsSudip,
am6010,
arpl,
cisse21,
debanjan97,
krishna2020,
lvrach and
ssbeefeater
as code owners
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
test/validator/validator.test.ts (1)
1111-1132: Test only triggers one rule violation despite the describe block name.The test is named "Multiple rule violations" but only triggers the secretKeys rule. The device/hybrid rule passes because
supportedConnectionModesis undefined. Consider adding a test that actually triggers both rules simultaneously.describe('Multiple rule violations', () => { - it('should collect all validation errors', async () => { + it('should collect all validation errors when multiple rules fail', async () => { const invalidDestDef = { name: 'TEST', displayName: 'Test', config: { includeKeys: ['apiKey', 'password'], secretKeys: ['password', 'token'], + supportedConnectionModes: { + web: ['device'], + }, }, }; + // This config violates the secretKeys rule (password in includeKeys) + // but satisfies the device/hybrid rule (includeKeys is defined) try { await validateDestinationDefinitions(invalidDestDef); fail('Expected validation to throw'); } catch (error) { const errorMessage = error.message; - // Should fail because password is in includeKeys but not in excludeKeys expect(errorMessage).toContain('Secret keys must not be exposed to client-side'); expect(errorMessage).toContain('password'); } }); + + it('should report both rule violations when both fail', async () => { + const invalidDestDef = { + name: 'TEST', + displayName: 'Test', + config: { + includeKeys: [], // Empty - violates device/hybrid rule + secretKeys: ['password'], + supportedConnectionModes: { + web: ['device'], + }, + }, + }; + + try { + await validateDestinationDefinitions(invalidDestDef); + fail('Expected validation to throw'); + } catch (error) { + const errorMessage = error.message; + // Empty includeKeys means secretKeys rule passes, but device/hybrid rule fails + expect(errorMessage).toContain('includeKeys must be defined and non-empty'); + } + }); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (76)
package.json(1 hunks)src/configurations/destinations/active_campaign/db-config.json(1 hunks)src/configurations/destinations/adj/db-config.json(1 hunks)src/configurations/destinations/af/db-config.json(0 hunks)src/configurations/destinations/am/db-config.json(1 hunks)src/configurations/destinations/appcenter/db-config.json(1 hunks)src/configurations/destinations/awin/db-config.json(0 hunks)src/configurations/destinations/axeptio/db-config.json(1 hunks)src/configurations/destinations/bingads/db-config.json(1 hunks)src/configurations/destinations/braze/db-config.json(1 hunks)src/configurations/destinations/bugsnag/db-config.json(1 hunks)src/configurations/destinations/candu/db-config.json(1 hunks)src/configurations/destinations/canny/db-config.json(0 hunks)src/configurations/destinations/clevertap/db-config.json(1 hunks)src/configurations/destinations/clickup/db-config.json(0 hunks)src/configurations/destinations/commandbar/db-config.json(1 hunks)src/configurations/destinations/comscore/db-config.json(1 hunks)src/configurations/destinations/convertflow/db-config.json(1 hunks)src/configurations/destinations/criteo/db-config.json(1 hunks)src/configurations/destinations/custify/db-config.json(0 hunks)src/configurations/destinations/customerio/db-config.json(1 hunks)src/configurations/destinations/dcm_floodlight/db-config.json(1 hunks)src/configurations/destinations/engage/db-config.json(1 hunks)src/configurations/destinations/facebook_offline_conversions/db-config.json(0 hunks)src/configurations/destinations/freshmarketer/db-config.json(0 hunks)src/configurations/destinations/freshsales/db-config.json(0 hunks)src/configurations/destinations/gainsight_px/db-config.json(1 hunks)src/configurations/destinations/iterable/db-config.json(1 hunks)src/configurations/destinations/june/db-config.json(1 hunks)src/configurations/destinations/kissmetrics/db-config.json(1 hunks)src/configurations/destinations/klaviyo_bulk_upload/db-config.json(0 hunks)src/configurations/destinations/kochava/db-config.json(1 hunks)src/configurations/destinations/leanplum/db-config.json(1 hunks)src/configurations/destinations/lemnisk/db-config.json(1 hunks)src/configurations/destinations/livechat/db-config.json(1 hunks)src/configurations/destinations/mailjet/db-config.json(0 hunks)src/configurations/destinations/mailmodo/db-config.json(0 hunks)src/configurations/destinations/marketo_static_list/db-config.json(0 hunks)src/configurations/destinations/matomo/db-config.json(1 hunks)src/configurations/destinations/mautic/db-config.json(0 hunks)src/configurations/destinations/mouseflow/db-config.json(1 hunks)src/configurations/destinations/mp/db-config.json(1 hunks)src/configurations/destinations/new_relic/db-config.json(0 hunks)src/configurations/destinations/pagerduty/db-config.json(0 hunks)src/configurations/destinations/pendo/db-config.json(1 hunks)src/configurations/destinations/persistiq/db-config.json(1 hunks)src/configurations/destinations/podsights/db-config.json(1 hunks)src/configurations/destinations/posthog/db-config.json(1 hunks)src/configurations/destinations/profitwell/db-config.json(1 hunks)src/configurations/destinations/qualaroo/db-config.json(1 hunks)src/configurations/destinations/qualtrics/db-config.json(1 hunks)src/configurations/destinations/quora_pixel/db-config.json(1 hunks)src/configurations/destinations/rakuten/db-config.json(1 hunks)src/configurations/destinations/reddit_pixel/db-config.json(1 hunks)src/configurations/destinations/refiner/db-config.json(0 hunks)src/configurations/destinations/rollbar/db-config.json(1 hunks)src/configurations/destinations/satismeter/db-config.json(1 hunks)src/configurations/destinations/sendinblue/db-config.json(1 hunks)src/configurations/destinations/sentry/db-config.json(1 hunks)src/configurations/destinations/singular/db-config.json(1 hunks)src/configurations/destinations/smartly/db-config.json(0 hunks)src/configurations/destinations/snap_pixel/db-config.json(1 hunks)src/configurations/destinations/snapchat_custom_audience/db-config.json(0 hunks)src/configurations/destinations/snapengage/db-config.json(1 hunks)src/configurations/destinations/spotifyPixel/db-config.json(1 hunks)src/configurations/destinations/stormly/db-config.json(1 hunks)src/configurations/destinations/tiktok_ads/db-config.json(1 hunks)src/configurations/destinations/user/db-config.json(0 hunks)src/configurations/destinations/userpilot/db-config.json(0 hunks)src/configurations/destinations/wootric/db-config.json(0 hunks)src/configurations/destinations/xpixel/db-config.json(1 hunks)src/configurations/destinations/yandex_metrica/db-config.json(1 hunks)src/configurations/destinations/zapier/db-config.json(0 hunks)src/schemas/destinations/db-config-schema.json(2 hunks)src/validator/index.ts(2 hunks)test/validator/validator.test.ts(1 hunks)
💤 Files with no reviewable changes (22)
- src/configurations/destinations/custify/db-config.json
- src/configurations/destinations/marketo_static_list/db-config.json
- src/configurations/destinations/freshmarketer/db-config.json
- src/configurations/destinations/refiner/db-config.json
- src/configurations/destinations/mailmodo/db-config.json
- src/configurations/destinations/pagerduty/db-config.json
- src/configurations/destinations/facebook_offline_conversions/db-config.json
- src/configurations/destinations/mautic/db-config.json
- src/configurations/destinations/userpilot/db-config.json
- src/configurations/destinations/klaviyo_bulk_upload/db-config.json
- src/configurations/destinations/smartly/db-config.json
- src/configurations/destinations/user/db-config.json
- src/configurations/destinations/canny/db-config.json
- src/configurations/destinations/freshsales/db-config.json
- src/configurations/destinations/awin/db-config.json
- src/configurations/destinations/zapier/db-config.json
- src/configurations/destinations/mailjet/db-config.json
- src/configurations/destinations/snapchat_custom_audience/db-config.json
- src/configurations/destinations/wootric/db-config.json
- src/configurations/destinations/clickup/db-config.json
- src/configurations/destinations/new_relic/db-config.json
- src/configurations/destinations/af/db-config.json
🧰 Additional context used
🧠 Learnings (9)
📓 Common learnings
Learnt from: shrouti1507
Repo: rudderlabs/rudder-integrations-config PR: 1732
File: test/data/validation/destinations/salesforce_oauth_sandbox.json:1-22
Timestamp: 2024-10-08T12:13:04.607Z
Learning: In the `rudder-integrations-config` repository, for closed testing destinations, identical test cases without consolidation are acceptable.
Learnt from: sandeepdsvs
Repo: rudderlabs/rudder-integrations-config PR: 1830
File: src/configurations/destinations/http/ui-config.json:476-477
Timestamp: 2024-12-05T16:06:11.176Z
Learning: The regex patterns in `src/configurations/destinations/http/ui-config.json` for the "Destination property" and "RudderStack property" fields correctly validate JSON paths and appropriately throw errors for invalid inputs.
📚 Learning: 2024-12-09T04:17:16.931Z
Learnt from: accoilmj
Repo: rudderlabs/rudder-integrations-config PR: 1807
File: src/configurations/destinations/accoil_analytics/schema.json:285-795
Timestamp: 2024-12-09T04:17:16.931Z
Learning: The file `src/configurations/destinations/accoil_analytics/schema.json` is autogenerated.
Applied to files:
src/configurations/destinations/satismeter/db-config.jsonsrc/configurations/destinations/commandbar/db-config.jsonsrc/configurations/destinations/sendinblue/db-config.jsonsrc/configurations/destinations/singular/db-config.jsonsrc/configurations/destinations/customerio/db-config.jsonsrc/configurations/destinations/clevertap/db-config.jsonsrc/configurations/destinations/bingads/db-config.jsonsrc/configurations/destinations/mouseflow/db-config.jsonsrc/configurations/destinations/gainsight_px/db-config.jsonsrc/configurations/destinations/qualaroo/db-config.jsonsrc/configurations/destinations/yandex_metrica/db-config.jsonsrc/configurations/destinations/leanplum/db-config.jsonsrc/configurations/destinations/snapengage/db-config.jsonsrc/configurations/destinations/active_campaign/db-config.jsonsrc/configurations/destinations/livechat/db-config.jsonsrc/configurations/destinations/braze/db-config.jsonsrc/schemas/destinations/db-config-schema.jsonsrc/configurations/destinations/mp/db-config.jsonsrc/configurations/destinations/persistiq/db-config.jsonsrc/configurations/destinations/posthog/db-config.jsonsrc/configurations/destinations/criteo/db-config.jsonsrc/configurations/destinations/snap_pixel/db-config.jsonsrc/configurations/destinations/podsights/db-config.jsonsrc/configurations/destinations/profitwell/db-config.jsonsrc/configurations/destinations/iterable/db-config.jsonsrc/configurations/destinations/comscore/db-config.jsonsrc/configurations/destinations/candu/db-config.jsonsrc/configurations/destinations/pendo/db-config.jsonsrc/configurations/destinations/june/db-config.jsonsrc/configurations/destinations/stormly/db-config.jsonsrc/configurations/destinations/kissmetrics/db-config.jsonsrc/configurations/destinations/adj/db-config.jsonsrc/configurations/destinations/matomo/db-config.jsonsrc/configurations/destinations/quora_pixel/db-config.jsonsrc/configurations/destinations/lemnisk/db-config.jsonsrc/configurations/destinations/spotifyPixel/db-config.jsonsrc/configurations/destinations/kochava/db-config.jsonsrc/configurations/destinations/am/db-config.jsonsrc/configurations/destinations/convertflow/db-config.json
📚 Learning: 2024-12-09T04:17:28.675Z
Learnt from: accoilmj
Repo: rudderlabs/rudder-integrations-config PR: 1807
File: src/configurations/destinations/accoil_analytics/schema.json:148-284
Timestamp: 2024-12-09T04:17:28.675Z
Learning: The file `src/configurations/destinations/accoil_analytics/schema.json` is autogenerated. Avoid suggesting changes to autogenerated files.
Applied to files:
src/configurations/destinations/satismeter/db-config.jsonsrc/configurations/destinations/commandbar/db-config.jsonsrc/configurations/destinations/sendinblue/db-config.jsonsrc/configurations/destinations/rakuten/db-config.jsonsrc/configurations/destinations/singular/db-config.jsonsrc/configurations/destinations/customerio/db-config.jsonsrc/configurations/destinations/clevertap/db-config.jsonsrc/configurations/destinations/bingads/db-config.jsonsrc/configurations/destinations/mouseflow/db-config.jsonsrc/configurations/destinations/gainsight_px/db-config.jsonsrc/configurations/destinations/tiktok_ads/db-config.jsonsrc/configurations/destinations/qualaroo/db-config.jsonsrc/configurations/destinations/yandex_metrica/db-config.jsonsrc/configurations/destinations/leanplum/db-config.jsonsrc/configurations/destinations/snapengage/db-config.jsonsrc/configurations/destinations/active_campaign/db-config.jsonsrc/configurations/destinations/livechat/db-config.jsonsrc/configurations/destinations/braze/db-config.jsonsrc/schemas/destinations/db-config-schema.jsonsrc/configurations/destinations/mp/db-config.jsonsrc/configurations/destinations/persistiq/db-config.jsonsrc/configurations/destinations/posthog/db-config.jsonsrc/configurations/destinations/criteo/db-config.jsonsrc/configurations/destinations/rollbar/db-config.jsonsrc/configurations/destinations/snap_pixel/db-config.jsonsrc/configurations/destinations/xpixel/db-config.jsonsrc/configurations/destinations/podsights/db-config.jsonsrc/configurations/destinations/profitwell/db-config.jsonsrc/configurations/destinations/dcm_floodlight/db-config.jsonsrc/configurations/destinations/iterable/db-config.jsonsrc/configurations/destinations/comscore/db-config.jsonsrc/configurations/destinations/candu/db-config.jsonsrc/configurations/destinations/pendo/db-config.jsonsrc/configurations/destinations/june/db-config.jsonsrc/configurations/destinations/stormly/db-config.jsonsrc/configurations/destinations/bugsnag/db-config.jsonsrc/configurations/destinations/kissmetrics/db-config.jsonsrc/configurations/destinations/adj/db-config.jsonsrc/configurations/destinations/matomo/db-config.jsonsrc/configurations/destinations/quora_pixel/db-config.jsonsrc/configurations/destinations/appcenter/db-config.jsonsrc/configurations/destinations/lemnisk/db-config.jsonsrc/configurations/destinations/spotifyPixel/db-config.jsonsrc/configurations/destinations/kochava/db-config.jsonsrc/configurations/destinations/am/db-config.jsonsrc/configurations/destinations/sentry/db-config.jsonsrc/configurations/destinations/convertflow/db-config.jsonsrc/configurations/destinations/engage/db-config.json
📚 Learning: 2024-12-09T04:17:05.013Z
Learnt from: accoilmj
Repo: rudderlabs/rudder-integrations-config PR: 1807
File: src/configurations/destinations/accoil_analytics/schema.json:11-147
Timestamp: 2024-12-09T04:17:05.013Z
Learning: The file `src/configurations/destinations/accoil_analytics/schema.json` is autogenerated and should not be manually modified.
Applied to files:
src/configurations/destinations/satismeter/db-config.jsonsrc/configurations/destinations/commandbar/db-config.jsonsrc/configurations/destinations/sendinblue/db-config.jsonsrc/configurations/destinations/rakuten/db-config.jsonsrc/configurations/destinations/singular/db-config.jsonsrc/configurations/destinations/customerio/db-config.jsonsrc/configurations/destinations/clevertap/db-config.jsonsrc/configurations/destinations/bingads/db-config.jsonsrc/configurations/destinations/mouseflow/db-config.jsonsrc/configurations/destinations/gainsight_px/db-config.jsonsrc/configurations/destinations/qualaroo/db-config.jsonsrc/configurations/destinations/yandex_metrica/db-config.jsonsrc/configurations/destinations/leanplum/db-config.jsonsrc/configurations/destinations/snapengage/db-config.jsonsrc/configurations/destinations/active_campaign/db-config.jsonsrc/configurations/destinations/livechat/db-config.jsonsrc/configurations/destinations/braze/db-config.jsonsrc/schemas/destinations/db-config-schema.jsonsrc/configurations/destinations/mp/db-config.jsonsrc/configurations/destinations/persistiq/db-config.jsonsrc/configurations/destinations/posthog/db-config.jsonsrc/configurations/destinations/criteo/db-config.jsonsrc/configurations/destinations/snap_pixel/db-config.jsonsrc/configurations/destinations/podsights/db-config.jsonsrc/configurations/destinations/profitwell/db-config.jsonsrc/configurations/destinations/iterable/db-config.jsonsrc/configurations/destinations/comscore/db-config.jsonsrc/configurations/destinations/candu/db-config.jsonsrc/configurations/destinations/pendo/db-config.jsonsrc/configurations/destinations/june/db-config.jsonsrc/configurations/destinations/stormly/db-config.jsonsrc/configurations/destinations/kissmetrics/db-config.jsonsrc/configurations/destinations/adj/db-config.jsonsrc/configurations/destinations/matomo/db-config.jsonsrc/configurations/destinations/quora_pixel/db-config.jsonsrc/configurations/destinations/lemnisk/db-config.jsonsrc/configurations/destinations/spotifyPixel/db-config.jsonsrc/configurations/destinations/kochava/db-config.jsonsrc/configurations/destinations/am/db-config.jsonsrc/configurations/destinations/convertflow/db-config.jsonsrc/configurations/destinations/engage/db-config.json
📚 Learning: 2024-10-15T07:02:58.743Z
Learnt from: vamsikrishnakandi
Repo: rudderlabs/rudder-integrations-config PR: 1751
File: src/configurations/destinations/rs/ui-config.json:219-231
Timestamp: 2024-10-15T07:02:58.743Z
Learning: In `src/configurations/destinations/rs/ui-config.json`, the `sshPublicKey` field is populated by the web application, even though it is marked as required and read-only in the UI configuration.
Applied to files:
src/configurations/destinations/satismeter/db-config.jsonsrc/configurations/destinations/commandbar/db-config.jsonsrc/configurations/destinations/singular/db-config.jsonsrc/configurations/destinations/mouseflow/db-config.jsonsrc/configurations/destinations/qualaroo/db-config.jsonsrc/configurations/destinations/snapengage/db-config.jsonsrc/configurations/destinations/braze/db-config.jsonsrc/configurations/destinations/june/db-config.jsonsrc/configurations/destinations/appcenter/db-config.jsonsrc/configurations/destinations/lemnisk/db-config.jsonsrc/configurations/destinations/engage/db-config.json
📚 Learning: 2024-10-08T05:19:11.373Z
Learnt from: saikumarrs
Repo: rudderlabs/rudder-integrations-config PR: 1691
File: src/configurations/destinations/ga4/schema.json:345-345
Timestamp: 2024-10-08T05:19:11.373Z
Learning: When reviewing changes to `src/configurations/destinations/ga4/schema.json`, ensure that added enum values like "iubenda" are correctly identified across all supported platforms before flagging an issue.
Applied to files:
src/configurations/destinations/rakuten/db-config.jsonsrc/configurations/destinations/bingads/db-config.jsonsrc/configurations/destinations/yandex_metrica/db-config.jsonsrc/configurations/destinations/active_campaign/db-config.jsonsrc/schemas/destinations/db-config-schema.jsonsrc/configurations/destinations/mp/db-config.jsonsrc/configurations/destinations/persistiq/db-config.jsonsrc/configurations/destinations/comscore/db-config.jsonsrc/configurations/destinations/candu/db-config.jsonsrc/configurations/destinations/stormly/db-config.jsonsrc/configurations/destinations/kissmetrics/db-config.jsonsrc/configurations/destinations/adj/db-config.jsontest/validator/validator.test.tssrc/configurations/destinations/matomo/db-config.jsonsrc/validator/index.tssrc/configurations/destinations/convertflow/db-config.json
📚 Learning: 2025-05-23T09:56:13.947Z
Learnt from: ItsSudip
Repo: rudderlabs/rudder-integrations-config PR: 2031
File: src/configurations/destinations/zoho/db-config.json:5-5
Timestamp: 2025-05-23T09:56:13.947Z
Learning: In the Rudder Integrations Config repository, flags like `isAudienceSupported` in destination configurations (`db-config.json` files) don't require schema updates when they're used as internal feature flags rather than user-configurable inputs.
Applied to files:
src/configurations/destinations/active_campaign/db-config.jsonsrc/schemas/destinations/db-config-schema.jsonsrc/configurations/destinations/mp/db-config.jsonsrc/configurations/destinations/comscore/db-config.json
📚 Learning: 2025-05-28T06:42:55.613Z
Learnt from: manish339k
Repo: rudderlabs/rudder-integrations-config PR: 2034
File: src/configurations/destinations/marketo_bulk_upload/schema.json:811-825
Timestamp: 2025-05-28T06:42:55.613Z
Learning: In Marketo bulk upload destination schema (src/configurations/destinations/marketo_bulk_upload/schema.json), the columnFieldsMapping items should NOT require "from" and "to" fields - they are intentionally optional in the schema design.
Applied to files:
src/schemas/destinations/db-config-schema.json
📚 Learning: 2024-12-05T16:06:11.176Z
Learnt from: sandeepdsvs
Repo: rudderlabs/rudder-integrations-config PR: 1830
File: src/configurations/destinations/http/ui-config.json:476-477
Timestamp: 2024-12-05T16:06:11.176Z
Learning: The regex patterns in `src/configurations/destinations/http/ui-config.json` for the "Destination property" and "RudderStack property" fields correctly validate JSON paths and appropriately throw errors for invalid inputs.
Applied to files:
test/validator/validator.test.tssrc/validator/index.ts
🧬 Code graph analysis (1)
test/validator/validator.test.ts (1)
src/validator/index.ts (1)
validateDestinationDefinitions(198-229)
🔇 Additional comments (63)
src/configurations/destinations/iterable/db-config.json (1)
175-175: Approved: No issues found.The change correctly removes
apiKeyfromsecretKeys. The schema showsapiKeyis required at the root level, and since web supports device mode, removing it fromsecretKeysaligns with the PR objective.The remaining field
registerDeviceOrBrowserApiKeyshould remain marked secret—it is not required by device-mode integrations. The schema defines it as an optional property with no conditional requirement whenconnectionMode.webequals "device". OnlyapiKeyanddataCenterare required at the root level, and the device-mode condition only requirespackageName(per theanyOfrule). The PR objective applies only to required fields, not optional ones.src/configurations/destinations/satismeter/db-config.json (1)
51-51: Correct application of secret field exposure prevention.The removal of
writeKeyfrom secretKeys is consistent with the PR objectives:writeKeyis included in the device-mode integration's exposed keys, so marking it as secret violates the rule that secret fields cannot be required by device-mode integrations.src/configurations/destinations/pendo/db-config.json (1)
36-36: Correct application of secret field exposure prevention.The removal of
apiKeyfrom secretKeys is correct:apiKeyis in the includeKeys exposed to device-mode, so it cannot be marked as secret per the PR rules.src/configurations/destinations/tiktok_ads/db-config.json (1)
133-133: Correct selective secret field removal.The removal of
pixelCodefrom secretKeys while retainingaccessTokenis correct:pixelCodeis in includeKeys and required for device-mode (web), whileaccessTokenis not in includeKeys, so it properly remains marked as secret.src/configurations/destinations/livechat/db-config.json (1)
49-49: Correct application of secret field exposure prevention.The removal of
licenseIdfrom secretKeys is correct:licenseIdis in includeKeys and required for device-mode integration, so it cannot be marked as secret.src/configurations/destinations/active_campaign/db-config.json (1)
130-130: Correct selective secret field removal.The removal of
actidfrom secretKeys while retainingapiKeyandeventKeyis correct:actidis in includeKeys and required for device/hybrid-mode, whileapiKeyandeventKeyare not in includeKeys, so they properly remain marked as secret.src/configurations/destinations/reddit_pixel/db-config.json (1)
43-43: Correct application of secret field exposure prevention.The removal of
advertiserIdfrom secretKeys is correct:advertiserIdis in includeKeys and required for device-mode integration, so it cannot be marked as secret.src/configurations/destinations/xpixel/db-config.json (1)
43-43: Correct application of secret field exposure prevention.The removal of
pixelIdfrom secretKeys is correct:pixelIdis in includeKeys and required for device-mode integration, so it cannot be marked as secret.src/configurations/destinations/convertflow/db-config.json (1)
47-47: Correct application of secret field exposure prevention.The removal of
websiteIdfrom secretKeys is correct:websiteIdis in includeKeys and required for device-mode integration, so it cannot be marked as secret.src/configurations/destinations/sentry/db-config.json (1)
61-61: ✓ Correct fix for secret exposure violation.Removing
dsnfromsecretKeysis correct since it's included inincludeKeysand device-mode is supported, making it automatically exposed in source configuration responses anyway.package.json (1)
21-21: ✓ Minor utility script addition.The
cleanscript is a convenience utility to removenode_modules. No functional impact on the PR objectives.src/configurations/destinations/engage/db-config.json (1)
128-128: ✓ Correct fix for secret exposure violation.Removing
privateKeyfromsecretKeysis appropriate since it's inincludeKeysand device-mode is supported. Secrets exposed viaincludeKeysdefeat the purpose of marking them secret.src/configurations/destinations/quora_pixel/db-config.json (1)
43-43: ✓ Correct fix for secret exposure violation.Removing
pixelIdfromsecretKeysis the right fix since it's inincludeKeyswith device-mode support, making it automatically exposed.src/configurations/destinations/appcenter/db-config.json (1)
86-86: ✓ Correct fix for secret exposure violation.Removing
appSecretfromsecretKeysis appropriate since it's inincludeKeysand device-mode is supported across multiple platforms.src/configurations/destinations/bugsnag/db-config.json (1)
56-56: ✓ Correct fix for secret exposure violation.Removing
apiKeyfromsecretKeysis the right approach since it's inincludeKeyswith device-mode support, making it exposed in source configurations.src/configurations/destinations/gainsight_px/db-config.json (1)
131-131: ✓ Correct selective fix for partial secret exposure violation.This change properly distinguishes between:
productTagKey(removed fromsecretKeysbecause it's inincludeKeysand exposed)apiKey(kept insecretKeysbecause it's NOT inincludeKeysand remains hidden)This selective approach correctly addresses only the actual violation.
src/configurations/destinations/adj/db-config.json (1)
141-141: ✓ Correct fix for secret exposure violation.Removing
appTokenfromsecretKeysis appropriate since it's inincludeKeysand device-mode is supported across multiple platforms (android, ios, flutter, unity).src/configurations/destinations/singular/db-config.json (1)
130-130: ✓ Correct handling of device-mode-required credentials.Removing
apiKeyandapiSecretfromsecretKeyswhile keeping them inincludeKeysaligns with the PR objective: fields required by device-mode integrations (android, ios, reactnative, cordova) cannot be marked secret since device SDKs need direct access.src/configurations/destinations/kochava/db-config.json (1)
129-129: ✓ Correct exposure of device-required credential.Clearing
secretKeysforapiKeyis correct, as device modes (android, ios, flutter) require it. Keeping it inincludeKeysensures it's available via the source config endpoint.src/configurations/destinations/posthog/db-config.json (1)
142-142: ✓ Correct handling of device-accessible credential.
teamApiKeyis appropriately removed fromsecretKeyssince web platform supports device mode and requires this credential. It remains inincludeKeysfor proper exposure.src/configurations/destinations/yandex_metrica/db-config.json (1)
55-55: ✓ Appropriate de-secreting of device-only credential.
tagIdcorrectly removed fromsecretKeyssince web (the only supported source type) exclusively uses device mode and requires this field. Remains inincludeKeys.src/configurations/destinations/dcm_floodlight/db-config.json (1)
143-143: ✓ Correct de-secreting for device-mode support.Removing
advertiserIdfromsecretKeysis correct since web platform supports device mode and needs this credential. Kept inincludeKeysfor proper exposure.src/configurations/destinations/stormly/db-config.json (1)
7-7: ✓ Correct handling of cloud-only secret.Removing
apiKeyfromincludeKeyswhile keeping it insecretKeysis correct. Since Stormly only supports cloud mode (no device/hybrid),apiKeyis a true server-side secret and should not be exposed viaincludeKeys.src/configurations/destinations/profitwell/db-config.json (1)
127-127: ✓ Correct differentiation between public and private credentials.Removing
publicApiKeyfromsecretKeysis appropriate since web platform supports device mode and requires this credential.privateApiKeycorrectly remains secret as it's only used server-side.src/configurations/destinations/clevertap/db-config.json (1)
138-138: ✓ Correct de-secreting of device-required credentials.Clearing
secretKeysis correct since multiple platforms support device mode (android, web, ios, reactnative) and requireaccountTokenandpasscodefrom defaultConfig. These fields remain inincludeKeysfor proper exposure.src/configurations/destinations/bingads/db-config.json (1)
44-44: Changes correctly align with secret field constraints.Removing
tagIDfrom secretKeys is correct becausetagIDis inincludeKeysand device mode is supported. Secret fields cannot be exposed to client-side SDKs.src/configurations/destinations/customerio/db-config.json (1)
144-144: Correct removal of apiKey from secretKeys.
apiKeyis inincludeKeysand device modes are supported for multiple source types (web, android, ios). Secret fields cannot be exposed to client-side SDKs, so this change is correct.src/configurations/destinations/sendinblue/db-config.json (1)
132-132: Correct selective updating of secretKeys.This change properly distinguishes between fields:
- Removed
clientKeyfrom secretKeys because it's inincludeKeys(exposed to device SDK)- Retained
apiKeyas secret because it's NOT inincludeKeys(not exposed to client)This correctly applies the rule that only truly secret (non-exposed) fields should be marked as secret.
src/configurations/destinations/qualaroo/db-config.json (1)
51-51: Correct emptying of secretKeys for exposed fields.Both
customerIdandsiteTokenare inincludeKeysand device mode is supported. These cannot be marked as secret since they're exposed to the client-side SDK.src/configurations/destinations/podsights/db-config.json (1)
45-45: Correct removal of pixelId from secretKeys.
pixelIdis inincludeKeysand device mode is supported. Since it's exposed to the client-side SDK, it cannot be marked as secret.src/configurations/destinations/snapengage/db-config.json (1)
45-45: Correct removal of widgetId from secretKeys.
widgetIdis inincludeKeysand device mode is supported. Since it's exposed to the client-side SDK, it cannot be marked as secret.src/configurations/destinations/leanplum/db-config.json (1)
148-148: Correct removal of clientKey from secretKeys.
clientKeyis inincludeKeysand device/hybrid modes are supported for multiple source types. Since it's exposed to client-side SDKs, it cannot be marked as secret.src/configurations/destinations/kissmetrics/db-config.json (1)
126-126: Correct removal of apiKey from secretKeys.
apiKeyis inincludeKeysand device mode is supported. Since it's exposed to the client-side SDK, it cannot be marked as secret.src/configurations/destinations/persistiq/db-config.json (1)
7-7: Removal of apiKey from includeKeys is correct.Since persistiq has no device-mode integrations and apiKey is marked as secret (line 107), removing it from includeKeys correctly enforces the rule that truly secret fields must not be included in includeKeys.
src/configurations/destinations/june/db-config.json (1)
119-119: Correct removal of apiKey from secretKeys.Since June's web integration supports device mode (line 37) and apiKey is in includeKeys (line 8) and defaultConfig (line 50), it cannot be marked as secret per the rule that secret fields must not be required by device-mode integrations.
src/configurations/destinations/lemnisk/db-config.json (1)
134-134: Correct removal of sdkWriteKey from secretKeys.Since sdkWriteKey is in includeKeys (line 8) and required by web's device mode configuration (line 66), it correctly cannot remain in secretKeys. The retained secret keys (apiKey, passKey, plWriteKey, diapiWriteKey) are used only in cloud/server-side configurations and appropriately remain secret.
src/configurations/destinations/spotifyPixel/db-config.json (1)
45-45: Correct removal of pixelId from secretKeys.SpotifyPixel supports device mode only (line 26), and pixelId is required for all modes (in defaultConfig at line 30 and includeKeys at line 8). It correctly cannot be marked as secret.
src/configurations/destinations/comscore/db-config.json (1)
72-72: Correct removal of publisherId from secretKeys.Comscore supports only device modes (android/device, ios/device, web/device), and publisherId is required for all modes (in defaultConfig at line 36 and includeKeys at line 7). It correctly cannot be marked as secret.
src/configurations/destinations/mouseflow/db-config.json (1)
41-41: Correct removal of websiteId from secretKeys.Mouseflow supports device mode only (line 24), and websiteId is required for all modes (in defaultConfig at line 28 and includeKeys at line 8). It correctly cannot be marked as secret.
src/configurations/destinations/axeptio/db-config.json (1)
43-43: Correct removal of clientId from secretKeys.Axeptio supports device mode only (line 25), and clientId is required for all modes (in defaultConfig at line 29 and includeKeys at line 8). It correctly cannot be marked as secret.
src/configurations/destinations/commandbar/db-config.json (1)
36-36: Correct removal of orgId from secretKeys.CommandBar supports device mode only (line 24), and orgId is required for all modes (in defaultConfig at line 27 and includeKeys at line 8). It correctly cannot be marked as secret.
src/configurations/destinations/rollbar/db-config.json (1)
55-55: Correctly removes accessToken from secretKeys.Since
accessTokenis included inincludeKeysand the destination supports device mode (web), marking it as secret would violate the PR constraint that fields required by device-mode integrations cannot be marked secret. This change aligns with the PR's objective to prevent secret exposure in device-mode contexts.src/configurations/destinations/mp/db-config.json (1)
186-186: Correctly refines secretKeys to exclude device-mode-exposed fields.The removal of
"token"fromsecretKeysis correct—tokenappears inincludeKeysand is required by device mode (web). Retaining onlygdprApiToken(which is properly excluded fromincludeKeys) maintains the PR's constraint that truly secret fields must not be inincludeKeys.src/configurations/destinations/qualtrics/db-config.json (1)
63-63: Correctly removes projectId from secretKeys for device-mode compliance.
projectIdis inincludeKeysand required by device-mode integrations (web, android, ios). Per the PR objective that fields required by device-mode integrations cannot be marked secret, emptyingsecretKeysis correct.src/configurations/destinations/criteo/db-config.json (1)
49-49: Correctly removes accountId from secretKeys for device-mode compliance.
accountIdis inincludeKeysand required by device mode (web). Per the PR constraint, it cannot be marked as a secret.src/configurations/destinations/snap_pixel/db-config.json (1)
49-49: Correctly removes pixelId from secretKeys for device-mode compliance.
pixelIdis inincludeKeysand required by device mode (web). Per the PR constraint, it cannot be marked as a secret.src/configurations/destinations/am/db-config.json (1)
209-209: Correctly refines secretKeys to maintain device-mode compliance and secret separation.Removing
apiKeyfromsecretKeysis correct—it is inincludeKeysand required by device mode integrations (web, android, ios, reactnative, flutter). RetainingapiSecret(which is properly absent fromincludeKeys) upholds the PR constraint that truly secret fields must not be exposed.src/configurations/destinations/braze/db-config.json (1)
176-176: Correctly refines secretKeys to maintain device-mode compliance and secret separation.Removing
appKeyfromsecretKeysis correct—it is inincludeKeysand required by multiple device-mode integrations. RetainingrestApiKey(which is properly absent fromincludeKeys) maintains the constraint that truly secret fields are not exposed via device-mode endpoints.src/configurations/destinations/candu/db-config.json (1)
7-7: Correctly removes apiKey from includeKeys for cloud-only destination.Since Candu only supports cloud-mode connections (no device or hybrid modes),
apiKeycan remain marked as a secret. Removing it fromincludeKeysprevents secret exposure via the source configuration endpoint while keeping it available indefaultConfigfor cloud-mode processing. This aligns with the PR objective.src/configurations/destinations/rakuten/db-config.json (1)
8-8: LGTM!Correctly removes
midfromincludeKeyssince it's marked as a secret insecretKeys(line 108). This prevents the merchant ID from being exposed via the source configuration response.src/configurations/destinations/matomo/db-config.json (1)
73-73: LGTM!Correctly clears
secretKeyssinceserverUrlandsiteIdare required for device-mode operation (line 40:"web": ["device"]). Per the PR objectives, fields required by device-mode integrations cannot be marked as secrets because they're automatically exposed via the source configuration endpoint.src/validator/index.ts (6)
19-23: LGTM!Clean and well-typed interface for validation rules with description for documentation and a validate function returning validation result with optional error message.
26-60: LGTM!The secretKeys validation rule is well-implemented with proper guards for undefined/null/empty arrays. The logic correctly allows secrets in
includeKeysonly when they're also inexcludeKeys(which takes precedence). The error message is actionable.
61-87: LGTM!The device/hybrid mode validation rule correctly identifies when at least one source type supports device or hybrid mode and enforces that
includeKeysmust be defined and non-empty in those cases. This ensures configurations sent to client SDKs are explicit.
88-138: Commented rules for future enforcement are reasonable.The TODO comments clearly indicate these rules will be enabled after cleaning up existing destination definitions. This approach allows incremental enforcement without breaking existing configurations.
140-153: LGTM!The error aggregation approach is consistent with the schema validation pattern (both use
JSON.stringifyfor error messages) and allows multiple rule violations to be reported in a single error.
225-227: LGTM!Custom rules are correctly applied after schema validation, ensuring the config structure is valid before semantic validation runs. This layered approach is clean and maintainable.
src/schemas/destinations/db-config-schema.json (2)
298-307: LGTM!The updated documentation for
includeKeysclearly explains the device/hybrid mode requirement and aligns with the validation rule enforced insrc/validator/index.ts.
309-318: LGTM!The updated documentation for
excludeKeysclarifies the filtering order (applied afterincludeKeys) and the behavior when keys overlap. This aligns with the validation logic in the secretKeys rule.test/validator/validator.test.ts (4)
876-893: LGTM!Minimal schema mock is appropriate for isolating custom validation rule testing from schema constraints. This allows the tests to focus on the custom rule logic.
895-1002: LGTM!Comprehensive test coverage for the secretKeys rule including:
- Basic pass/fail scenarios
excludeKeysoverride behavior (secret in both lists should pass)- Edge cases with empty and undefined arrays
- Multiple secret exposure detection
1004-1109: LGTM!Comprehensive test coverage for the device/hybrid mode rule including:
- Device mode and hybrid mode scenarios
- Missing vs empty
includeKeysdifferentiation- Cloud-only configurations (should pass without
includeKeys)- Undefined
supportedConnectionModeshandling- Multiple source types with mixed modes
1134-1158: LGTM!Edge case tests appropriately verify:
- Undefined config fails schema validation (config is required)
- Null config properties pass since
Array.isArray(null)returnsfalse, treating them as "not defined"This documents the defensive behavior of the validation rules.
|
This PR is considered to be stale. It has been open for 20 days with no further activity thus it is going to be closed in 7 days. To avoid such a case please consider removing the stale label manually or add a comment to the PR. |
|
This PR is considered to be stale. It has been open for 20 days with no further activity thus it is going to be closed in 7 days. To avoid such a case please consider removing the stale label manually or add a comment to the PR. |
3 participants
What are the changes introduced in this PR?
includeKeys.What is the related Linear task?
https://linear.app/rudderstack/issue/INT-4282/review-all-the-integration-definition-configurations-for-secret-fields
Please explain the objectives of your changes below
DO NOT expose secret keys in source configuration response (via includeKeys).
Any changes to existing capabilities/behaviour, mention the reason & what are the changes ?
Improved the validator to add custom rules beyond the JSON schema validation for destination definition configuration file.
Any new dependencies introduced with this change?
No
Any new checks got introduced or modified in test suites. Please explain the changes.
Destination definitions are now thoroughly validated to not expose secrets.
Developer checklist
My code follows the style guidelines of this project
No breaking changes are being introduced.
All related docs linked with the PR?
All changes manually tested?
Any documentation changes needed with this change?
I have executed schemaGenerator tests and updated schema if needed
Are sensitive fields marked as secret in definition config?
My test cases and placeholders use only masked/sample values for sensitive fields
Is the PR limited to 10 file changes & one task?
Reviewer checklist
Is the type of change in the PR title appropriate as per the changes?
Verified that there are no credentials or confidential data exposed with the changes.
Summary by CodeRabbit
Chores
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.