Skip to content

fix(connectors-sdk): BaseSettings serialization and security issues (#7100) - #7101

Open
Powlinett wants to merge 5 commits into
masterfrom
feat/7100-connectors-sdk-base-settings-fixes
Open

fix(connectors-sdk): BaseSettings serialization and security issues (#7100)#7101
Powlinett wants to merge 5 commits into
masterfrom
feat/7100-connectors-sdk-base-settings-fixes

Conversation

@Powlinett

Copy link
Copy Markdown
Member

Proposed changes

  • Use SecretStr type for OPENCTI_TOKEN env var to avoid leaking credentials in logs
  • Use field_serializer for CONNECTOR_SCOPE for consistency
  • Fix serialization issue by making deprecated fields optional (allow model_dump() with exclude_none)
  • Extract _SettingsLoader into its own dedicated module
  • Update downstream connector tests after OPENCTI_TOKEN type change

Related issues

Checklist

  • I consider the submitted work as finished
  • I have signed my commits using GPG key.
  • I tested the code for its functionality using different use cases
  • I added/update the relevant documentation (either on github or on notion)
  • Where necessary I refactored code to improve the overall quality

Further comments

This PR is part of a split from #6559. The logger module work continues in a stacked PR based on this branch.

Copilot AI review requested due to automatic review settings July 24, 2026 14:50
@Powlinett Powlinett added bug Type: something isn't working (fix:). connectors-sdk Linked to connectors-sdk. labels Jul 24, 2026
@github-actions

This comment was marked as off-topic.

@Filigran-Automation Filigran-Automation added filigran team Item from the Filigran team. and removed bug Type: something isn't working (fix:). connectors-sdk Linked to connectors-sdk. labels Jul 24, 2026
@github-actions

This comment was marked as off-topic.

@codecov

codecov Bot commented Jul 24, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@             Coverage Diff             @@
##           master    #7101       +/-   ##
===========================================
+ Coverage   35.64%   66.60%   +30.96%     
===========================================
  Files        2048     1308      -740     
  Lines      125310    70265    -55045     
===========================================
+ Hits        44672    46803     +2131     
+ Misses      80638    23462    -57176     
Files with missing lines Coverage Δ
...rs-sdk/connectors_sdk/settings/_settings_loader.py 100.00% <100.00%> (ø)
...ors-sdk/connectors_sdk/settings/annotated_types.py 100.00% <ø> (ø)
...ctors-sdk/connectors_sdk/settings/base_settings.py 100.00% <100.00%> (ø)

... and 1199 files with indirect coverage changes

📢 Thoughts on this report? Let us know!

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses serialization and secret-handling issues in connectors-sdk settings models by switching OPENCTI_TOKEN to SecretStr, normalizing CONNECTOR_SCOPE JSON serialization via field_serializer, making deprecated fields optional for safer dumping, and extracting _SettingsLoader into a dedicated module. It also updates downstream connector tests to reflect the new SecretStr behavior.

Changes:

  • Update BaseSettings/BaseConnectorSettings to treat OPENCTI_TOKEN as SecretStr and selectively reveal it only for pycti helper config output.
  • Standardize CONNECTOR_SCOPE serialization for pycti via field_serializer, and refactor _SettingsLoader into connectors_sdk.settings._settings_loader.
  • Update SDK and connector test suites/fixtures to align with new token typing and updated scope/log-level fixture values.

Reviewed changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
stream/microsoft-sentinel-intel/tests/microsoft_sentinel_intel/test_config.py Adjust expected OpenCTI token representation in dumped config.
internal-enrichment/qualys-cve-enrichment/tests/test_connector/test_settings.py Update token assertions for SecretStr.
internal-enrichment/polyswarm-sandbox/tests/test_connector.py Update token assertions for SecretStr.
internal-enrichment/censys-enrichment/tests/censys_enrichment/test_config.py Update token assertions for SecretStr.
external-import/flare/tests/test_connector/test_settings.py Ensure helper config contains clear token via get_secret_value().
external-import/ctm360-hackerview-feed/tests/test_connector/test_settings.py Update token assertions for SecretStr.
external-import/ctm360-cyna-feed/tests/test_connector/test_settings.py Update token assertions for SecretStr.
external-import/ctm360-cyberblindspot-feed/tests/test_connector/test_settings.py Update token assertions for SecretStr.
connectors-sdk/tests/test_settings/test_settings_loader.py Add dedicated tests for new _SettingsLoader module behavior.
connectors-sdk/tests/test_settings/test_deprecation_migration.py Update scope fixtures to match comma-separated behavior.
connectors-sdk/tests/test_settings/test_base_settings.py Update base settings tests for SecretStr token + scope serialization changes.
connectors-sdk/tests/test_settings/test_annotated_types.py Remove tests tied to ListFromString JSON serialization behavior that was moved.
connectors-sdk/tests/test_settings/data/config.test.yml Update config fixture scope/log-level values.
connectors-sdk/tests/test_settings/data/.env.test Update env fixture scope/log-level values.
connectors-sdk/tests/test_settings/conftest.py Point fixtures to _settings_loader module + update scope fixture.
connectors-sdk/connectors_sdk/settings/base_settings.py Implement SecretStr token, per-field serialization rules, deprecated-field optionality, and improved deprecated namespace handling.
connectors-sdk/connectors_sdk/settings/annotated_types.py Remove ListFromString JSON serializer now that scope serialization is handled in settings models.
connectors-sdk/connectors_sdk/settings/_settings_loader.py Extract _SettingsLoader into a dedicated module.

Comment thread connectors-sdk/connectors_sdk/settings/base_settings.py Outdated
Comment thread connectors-sdk/connectors_sdk/settings/base_settings.py
@Powlinett
Powlinett force-pushed the feat/7100-connectors-sdk-base-settings-fixes branch from e96aa2c to 62c844f Compare July 24, 2026 15:08
@Powlinett
Powlinett force-pushed the feat/7100-connectors-sdk-base-settings-fixes branch from 62c844f to f6908d0 Compare July 24, 2026 15:16
@Powlinett Powlinett added the connectors-sdk Linked to connectors-sdk. label Jul 24, 2026

@throuxel throuxel left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

connectors-sdk Linked to connectors-sdk. filigran team Item from the Filigran team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(connectors-sdk): BaseSettings serialization and security issues

5 participants