Skip to content

fix: mark secret adapter properties private - #71

Open
hamzahalq wants to merge 1 commit into
net6from
hamza/fix/private-adapter-secrets
Open

hamzahalq wants to merge 1 commit into
net6from
hamza/fix/private-adapter-secrets

Conversation

@hamzahalq

Copy link
Copy Markdown
Contributor

Bitween hides a subscription's adapter settings only when the adapter marks them private. These adapters didn't mark any, so opening a subscription returned passwords, API keys and connection strings in plain text to anyone who can view subscriptions. This came up in a client penetration test.

  • 20 settings marked private across 14 adapters: ApiKey, Password, ClientSecret, ConnectionString, SecretAccessKey and LicenseKey.
  • SDK raised to 6.0.26 in the 8 adapters still on 5.0.x, because 5.0.7 can't mark a setting private. 6.0.26 is what the repo's other 6.0 adapters already use. The SDK changes since 5.0.7 are the net6 move, formatting and the private flag itself.

Not marked: the HTTP Headers setting, which can carry tokens but would be hidden as a whole, and the MS Teams webhook Url.

Tested on a local Bitween with the HTTP handler. On the old version, opening the subscription returned the password. On the new version it returns __private__. Saving from the UI keeps the stored password. Replacing it in the UI stores and uses the new one. A subscription created on the old version keeps working after the upgrade. The solution builds. The HTTP unit tests fail the same 4 tests as on net6, because they call external URLs.

Deploying: nothing publishes these adapters automatically. Each one has to be published and uploaded to a deployment by hand.

Bitween masks only the properties an adapter declares private, and the flag needs SDK 6.0.x.
@coderabbitai

coderabbitai Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: simplify9/coderabbit/.coderabbit.yaml
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: ec1e8db4-dc73-450c-a106-d207c32598e8
📥 Commits

Reviewing files that changed from the base of the PR and between d6726b5 and dc30745.

📒 Files selected for processing (22)
  • SW.Infolink.Adapters.Handlers.Notifier.Sendgrid/Handler.cs
  • SW.Infolink.Adapters.Handlers.Notifier.Sendgrid/SW.Infolink.Adapters.Handlers.Notifier.Sendgrid.csproj
  • SW.InfolinkAdapters.Handlers.AzureBlob/Handler.cs
  • SW.InfolinkAdapters.Handlers.AzureBlob/SW.InfolinkAdapters.Handlers.AzureBlob.csproj
  • SW.InfolinkAdapters.Handlers.BulkSmtp/Handler.cs
  • SW.InfolinkAdapters.Handlers.Http/Handler.cs
  • SW.InfolinkAdapters.Handlers.Http/SW.InfolinkAdapters.Handlers.Http.csproj
  • SW.InfolinkAdapters.Handlers.Mailgun/Handler.cs
  • SW.InfolinkAdapters.Handlers.Mailgun/SW.InfolinkAdapters.Handlers.Mailgun.csproj
  • SW.InfolinkAdapters.Handlers.S3/Handler.cs
  • SW.InfolinkAdapters.Handlers.S3/SW.InfolinkAdapters.Handlers.S3.csproj
  • SW.InfolinkAdapters.Handlers.Sendgrid/Handler.cs
  • SW.InfolinkAdapters.Handlers.Sendgrid/SW.InfolinkAdapters.Handlers.Sendgrid.csproj
  • SW.InfolinkAdapters.Handlers.Smtp/Handler.cs
  • SW.InfolinkAdapters.Receivers.AzureBlob/Handler.cs
  • SW.InfolinkAdapters.Receivers.ElasticSearch/Handler.cs
  • SW.InfolinkAdapters.Receivers.Ftp/Handler.cs
  • SW.InfolinkAdapters.Receivers.Http/Handler.cs
  • SW.InfolinkAdapters.Receivers.Http/SW.InfolinkAdapters.Receivers.Http.csproj
  • SW.InfolinkAdapters.Receivers.Pop3/Handler.cs
  • SW.InfolinkAdapters.Receivers.Pop3/SW.InfolinkAdapters.Receivers.Pop3.csproj
  • SW.InfolinkAdapters.Receivers.S3/Handler.cs

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📜 Recent review details
🔇 Additional comments (21)
SW.Infolink.Adapters.Handlers.Notifier.Sendgrid/Handler.cs (1)

19-19: LGTM!

SW.Infolink.Adapters.Handlers.Notifier.Sendgrid/SW.Infolink.Adapters.Handlers.Notifier.Sendgrid.csproj (1)

14-14: LGTM!

SW.InfolinkAdapters.Handlers.AzureBlob/Handler.cs (1)

15-15: LGTM!

SW.InfolinkAdapters.Handlers.AzureBlob/SW.InfolinkAdapters.Handlers.AzureBlob.csproj (1)

11-11: LGTM!

SW.InfolinkAdapters.Handlers.BulkSmtp/Handler.cs (1)

14-14: LGTM!

SW.InfolinkAdapters.Handlers.Http/Handler.cs (1)

36-36: LGTM!

Also applies to: 39-39, 46-46

SW.InfolinkAdapters.Handlers.Http/SW.InfolinkAdapters.Handlers.Http.csproj (1)

14-14: LGTM!

SW.InfolinkAdapters.Handlers.Mailgun/Handler.cs (1)

18-18: LGTM!

SW.InfolinkAdapters.Handlers.Mailgun/SW.InfolinkAdapters.Handlers.Mailgun.csproj (1)

9-9: LGTM!

SW.InfolinkAdapters.Handlers.S3/Handler.cs (1)

14-14: LGTM!

SW.InfolinkAdapters.Handlers.S3/SW.InfolinkAdapters.Handlers.S3.csproj (1)

11-11: LGTM!

SW.InfolinkAdapters.Handlers.Sendgrid/Handler.cs (1)

18-18: LGTM!

SW.InfolinkAdapters.Handlers.Sendgrid/SW.InfolinkAdapters.Handlers.Sendgrid.csproj (1)

10-10: LGTM!

SW.InfolinkAdapters.Handlers.Smtp/Handler.cs (1)

14-14: LGTM!

SW.InfolinkAdapters.Receivers.ElasticSearch/Handler.cs (1)

19-19: LGTM!

SW.InfolinkAdapters.Receivers.Ftp/Handler.cs (1)

20-20: LGTM!

Also applies to: 24-24

SW.InfolinkAdapters.Receivers.Http/Handler.cs (1)

20-20: LGTM!

Also applies to: 23-23, 27-27

SW.InfolinkAdapters.Receivers.Http/SW.InfolinkAdapters.Receivers.Http.csproj (1)

9-9: LGTM!

SW.InfolinkAdapters.Receivers.Pop3/Handler.cs (1)

19-19: LGTM!

SW.InfolinkAdapters.Receivers.Pop3/SW.InfolinkAdapters.Receivers.Pop3.csproj (1)

11-11: LGTM!

SW.InfolinkAdapters.Receivers.AzureBlob/Handler.cs (1)

18-18: 🩺 Stability & Availability

The finding is refuted. In SDK 6.0.26, Runner.Expect takes optional before isPrivate and stores both values. Each false, true call keeps the credential non-optional and marks it private. The S3 client question is moot for this change.


📝 Summary

Summary

  • Marks credential settings as private through Runner.Expect across affected adapters. The changes cover API keys, passwords, client secrets, connection strings, secret access keys, and license keys.
  • Updates affected adapter projects to SimplyWorks.Serverless.Sdk 6.0.26, which supports private settings.

Risk

risk:medium — The change affects how credentials appear and persist in configuration UIs, and it upgrades the SDK used by affected adapters.

Security-sensitive areas

Credential masking and storage behavior. The change does not mark HTTP Headers or the MS Teams webhook Url as private, according to the PR description.

Test coverage impact

The PR description reports a successful build. It also reports four HTTP unit test failures caused by calls to external URLs; these are said to match failures on net6. No independent test results were provided.

Operational concerns

The PR description says affected adapters are not published automatically. Publish each adapter and upload it to the deployment manually. The description reports that existing subscriptions continue to work after upgrade and that UI saves preserve stored passwords, but these behaviors were not independently verified here.

Walkthrough

Outbound handlers and receivers update credential expectation arguments. Several adapter projects add or update their SimplyWorks.Serverless.Sdk reference to version 6.0.26.

Changes

Credential expectation updates

Layer / File(s) Summary
Outbound handler expectations
SW.Infolink.Adapters.Handlers.Notifier.Sendgrid/*, SW.InfolinkAdapters.Handlers.AzureBlob/*, SW.InfolinkAdapters.Handlers.BulkSmtp/Handler.cs, SW.InfolinkAdapters.Handlers.Http/*, SW.InfolinkAdapters.Handlers.Mailgun/*, SW.InfolinkAdapters.Handlers.S3/*, SW.InfolinkAdapters.Handlers.Sendgrid/*, SW.InfolinkAdapters.Handlers.Smtp/Handler.cs
Credential expectations gain or change arguments to Runner.Expect. The HTTP handler marks ApiKey, Password, and ClientSecret as sensitive. Several projects add or update the SDK reference to version 6.0.26.
Receiver credential expectations
SW.InfolinkAdapters.Receivers.AzureBlob/Handler.cs, SW.InfolinkAdapters.Receivers.ElasticSearch/Handler.cs, SW.InfolinkAdapters.Receivers.Ftp/Handler.cs, SW.InfolinkAdapters.Receivers.Http/*, SW.InfolinkAdapters.Receivers.Pop3/*, SW.InfolinkAdapters.Receivers.S3/Handler.cs
Receiver credential expectations change their optionality or sensitivity flags. The HTTP receiver marks ApiKey, Password, and ClientSecret as required. Several projects update the SDK reference to version 6.0.26.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested labels: security, risk:critical

Suggested reviewers: samerzughul

Merge Risk: ⚪ Minimal · up to dc307

The reviewed receiver credentials remain required and private. No concrete merge-blocking issue is established; HTTP Headers redaction could not be confirmed for the pinned SDK.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 16 functions across 14 files. (8 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly summarizes the change: adapter secret properties are now marked private.
Description check ✅ Passed The description explains the security issue, the private settings and SDK upgrades, and reports testing and deployment requirements.
Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 16 functions across 14 files. (8 skipped: 8 unsupported.)

  • Fix all pre-merge checks with AI
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants