Skip to content

fix: various fixes for log information, cryptomodule, deprecation warnings#503

Merged
mohitpubnub merged 5 commits into
masterfrom
CLEN-3613/scan-fixes
Jul 21, 2026
Merged

fix: various fixes for log information, cryptomodule, deprecation warnings#503
mohitpubnub merged 5 commits into
masterfrom
CLEN-3613/scan-fixes

Conversation

@mohitpubnub

@mohitpubnub mohitpubnub commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

refactor: omit potential use sensitive information from logs.

Logger omits potential use sensitive information to be logged.

refactor: added deprecation warning for legacy crypro module.

User will get warning while trying to configure legacy crypto module.

fix: encoding on user provided file api inputs to avoid potential path traversal attack.

encoding on user provided file api inputs to avoid potential path traversal attack.

Mohit Tejani added 4 commits July 16, 2026 11:37
…any potential path traversal attack.

added tests for the same wherever the user can provide file id/name inputs
…tions from being logged,

added deprection warning on LegacyCryptoModule to aware user of potential risk,
additional recommanation for using aesCbcCryptor for cryptography.
@coderabbitai

coderabbitai Bot commented Jul 20, 2026

Copy link
Copy Markdown

Important

Review skipped

Review was skipped due to path filters

⛔ Files ignored due to path filters (36)
  • .pubnub.yml is excluded by !.pubnub.yml and included by none
  • CHANGELOG.md is excluded by none and included by none
  • README.md is excluded by none and included by none
  • dist/web/pubnub.js is excluded by !**/dist/**, !dist/** and included by none
  • dist/web/pubnub.min.js is excluded by !**/dist/**, !**/*.min.js, !dist/** and included by none
  • lib/core/components/configuration.js is excluded by !lib/** and included by none
  • lib/core/components/cryptography/index.js is excluded by !lib/** and included by none
  • lib/core/endpoints/file_upload/delete_file.js is excluded by !lib/** and included by none
  • lib/core/endpoints/file_upload/download_file.js is excluded by !lib/** and included by none
  • lib/core/endpoints/file_upload/get_file_url.js is excluded by !lib/** and included by none
  • lib/core/interfaces/crypto-module.js is excluded by !lib/** and included by none
  • lib/core/pubnub-common.js is excluded by !lib/** and included by none
  • lib/core/utils.js is excluded by !lib/** and included by none
  • lib/crypto/modules/NodeCryptoModule/aesCbcCryptor.js is excluded by !lib/** and included by none
  • lib/crypto/modules/NodeCryptoModule/legacyCryptor.js is excluded by !lib/** and included by none
  • lib/crypto/modules/NodeCryptoModule/nodeCryptoModule.js is excluded by !lib/** and included by none
  • lib/types/index.d.ts is excluded by !lib/** and included by none
  • package-lock.json is excluded by !**/package-lock.json, !package-lock.json and included by none
  • package.json is excluded by none and included by none
  • src/core/components/configuration.ts is excluded by none and included by none
  • src/core/components/cryptography/index.ts is excluded by none and included by none
  • src/core/endpoints/file_upload/delete_file.ts is excluded by none and included by none
  • src/core/endpoints/file_upload/download_file.ts is excluded by none and included by none
  • src/core/endpoints/file_upload/get_file_url.ts is excluded by none and included by none
  • src/core/interfaces/configuration.ts is excluded by none and included by none
  • src/core/interfaces/crypto-module.ts is excluded by none and included by none
  • src/core/pubnub-common.ts is excluded by none and included by none
  • src/core/utils.ts is excluded by none and included by none
  • src/crypto/modules/NodeCryptoModule/aesCbcCryptor.ts is excluded by none and included by none
  • src/crypto/modules/NodeCryptoModule/legacyCryptor.ts is excluded by none and included by none
  • src/crypto/modules/NodeCryptoModule/nodeCryptoModule.ts is excluded by none and included by none
  • src/crypto/modules/WebCryptoModule/aesCbcCryptor.ts is excluded by none and included by none
  • src/crypto/modules/WebCryptoModule/legacyCryptor.ts is excluded by none and included by none
  • src/crypto/modules/WebCryptoModule/webCryptoModule.ts is excluded by none and included by none
  • test/integration/endpoints/get_file_url.test.ts is excluded by none and included by none
  • test/unit/file_upload/file_path_encoding.test.ts is excluded by none and included by none

CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including **/dist/** will override the default block on the dist directory, by removing the pattern from both the lists.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 13b0a3b7-a5f7-4806-aa07-cfcf7a6be5da

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch CLEN-3613/scan-fixes

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.

@pubnub-ops-terraform

pubnub-ops-terraform commented Jul 20, 2026

Copy link
Copy Markdown

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues
Licenses 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@mohitpubnub mohitpubnub self-assigned this Jul 20, 2026

@parfeon parfeon 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.

In general I have doubts that there were any "critical security risks" which has been addressed with fixes.

Comment on lines +60 to +65
/**
* Construct crypto module with legacy cryptor for encryption and both legacy and AES-CBC
* cryptors for decryption.
*
* @deprecated Use {@link aesCbcCryptoModule} for new applications.
*/

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.

Is there a need in this if it already properly deprecated in AbstractCryptoModule?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Same as above when used in TS

Comment on lines +64 to +69
/**
* Construct crypto module with legacy cryptor for encryption and both legacy and AES-CBC
* cryptors for decryption.
*
* @deprecated Use {@link aesCbcCryptoModule} for new applications.
*/

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.

Is there a need in this if it already properly deprecated in AbstractCryptoModule?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

For enduser, I did not see deprecation warning in intellisense when trying to configure legacy crypto module.

With this changes, I see that deprecation warning

earlier behaviour:
Image

after the changes (strikeout on legacyCryptoModule when try to initialise it at first step):

Image

Comment thread src/core/pubnub-common.ts
message: { token },
details: 'Revoke token permissions with parameters:',
}));
this.logger.debug('PubNub', 'Revoke token permissions.');

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.

There were reasons why we printed this all out for the engineer: so later they will share logs with us and we will see whether proper value has been passed for this or another API or not.
This change will make it harder debug issues related to the PAM.

@mohitpubnub

Copy link
Copy Markdown
Contributor Author

@pubnub-release-bot release

@mohitpubnub
mohitpubnub merged commit 8a2a7ee into master Jul 21, 2026
10 checks passed
@mohitpubnub
mohitpubnub deleted the CLEN-3613/scan-fixes branch July 21, 2026 09:46
@pubnub-release-bot

Copy link
Copy Markdown
Contributor

🚀 Release successfully completed 🚀

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.

4 participants