-
Notifications
You must be signed in to change notification settings - Fork 43
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add hostnameExceptionList for ssrf #100
Conversation
WalkthroughA new Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant Config as config.default.js
participant Context as context.js
participant Utils as utils.js
participant Test as ssrf.test.js
App->>Config: Read ssrf configuration
Config-->>App: Return ssrf settings with hostnameExceptionList
App->>Context: Access refererWhiteList from security.csrf
Context->>Utils: Call ssrf.checkAddress with family and hostname
Utils->>Config: Check hostname against hostnameExceptionList
Utils-->>Context: Return address check result
Test->>App: Mock curl and test safeCurl
App-->>Test: Return test results with valid hostnameExceptionList checks
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
65c852d
to
67c0043
Compare
Run & review this pull request in StackBlitz Codeflow. commit: egg-security
|
Run & review this pull request in StackBlitz Codeflow. commit: egg-security
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Outside diff range and nitpick comments (2)
test/fixtures/apps/ssrf-hostname-exception-list/config/config.default.js (1)
1-1
: Remove redundant 'use strict' directive.JavaScript modules are in strict mode by default, making this directive unnecessary.
- 'use strict';
Tools
Biome
[error] 1-1: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.(lint/suspicious/noRedundantUseStrict)
lib/utils.js (1)
114-121
: Change to an optional chain.Use optional chaining to simplify the hostname check.
- if (hostname && hostnameExceptionList) { - if (hostnameExceptionList.includes(hostname)) { - return true; - } + if (hostnameExceptionList?.includes(hostname)) { + return true;
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (7)
- README.md (2 hunks)
- app/extend/context.js (1 hunks)
- config/config.default.js (1 hunks)
- lib/utils.js (1 hunks)
- test/fixtures/apps/ssrf-hostname-exception-list/config/config.default.js (1 hunks)
- test/fixtures/apps/ssrf-hostname-exception-list/package.json (1 hunks)
- test/ssrf.test.js (1 hunks)
Files skipped from review due to trivial changes (1)
- test/fixtures/apps/ssrf-hostname-exception-list/package.json
Additional context used
Biome
test/fixtures/apps/ssrf-hostname-exception-list/config/config.default.js
[error] 1-1: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.(lint/suspicious/noRedundantUseStrict)
lib/utils.js
[error] 111-111: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
LanguageTool
README.md
[grammar] ~526-~526: Consider using third-person verb forms for singular and mass nouns.
Context: ...onList(Array) - specific which hostname are legal within ipBlackList. - checkAddres...(MASS_AGREEMENT)
Additional comments not posted (5)
test/fixtures/apps/ssrf-hostname-exception-list/config/config.default.js (1)
10-13
: Ensure hostname exception list is comprehensive.Review the list of hostnames to ensure all necessary exceptions are included.
config/config.default.js (1)
106-106
: EnsurehostnameExceptionList
is properly configured.Verify that the
hostnameExceptionList
is correctly populated and used within the application.test/ssrf.test.js (1)
175-204
: Ensure comprehensive testing ofhostnameExceptionList
.Verify that the test cases cover all possible scenarios for the
hostnameExceptionList
.app/extend/context.js (1)
212-212
: Verify the correctness of the new access path.The new access path for
refererWhiteList
isthis.app.config.security.csrf.refererWhiteList
. Ensure that this path is correct and that therefererWhiteList
property exists withincsrf
.Verification successful
The new access path is correct.
The
refererWhiteList
property is correctly accessed asthis.app.config.security.csrf.refererWhiteList
.
lib/utils.js
:config.csrf.refererWhiteList
config/config.default.js
:refererWhiteList
withincsrf
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the existence of `refererWhiteList` in `csrf`. # Test: Search for the configuration definition. Expect: `refererWhiteList` is defined within `csrf`. rg --type javascript $'refererWhiteList'Length of output: 74
Script:
#!/bin/bash # Description: Verify the existence of `refererWhiteList` in `csrf`. # Test: Search for the configuration definition. Expect: `refererWhiteList` is defined within `csrf`. rg --type js 'refererWhiteList'Length of output: 1331
README.md (1)
544-547
: Verify the validity of the hostnames in the example.Ensure that the hostnames provided in the
hostnameExceptionList
example are valid and correctly formatted.Verification successful
The hostnames in the
hostnameExceptionList
example are valid and correctly formatted.
registry.npmjs.org
registry.npmmirror.com
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the validity of hostnames in `hostnameExceptionList`. # Test: Check if the hostnames in the example are valid. Expect: Hostnames are correctly formatted. rg --type javascript $'hostnameExceptionList:'Length of output: 80
Script:
#!/bin/bash # Description: Verify the validity of hostnames in `hostnameExceptionList`. # Test: Check if the hostnames in the example are valid. Expect: Hostnames are correctly formatted. rg 'hostnameExceptionList:' --glob '*.js' --glob '*.jsx' --glob '*.ts' --glob '*.tsx'Length of output: 247
Script:
#!/bin/bash # Description: Display the contents of the file containing hostnameExceptionList. # Test: Display the lines around the hostnameExceptionList to verify the hostnames. rg 'hostnameExceptionList:' --glob 'test/fixtures/apps/ssrf-hostname-exception-list/config/config.default.js' -A 5 -B 5Length of output: 1109
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #100 +/- ##
==========================================
+ Coverage 97.26% 97.28% +0.01%
==========================================
Files 32 32
Lines 1354 1362 +8
Branches 340 342 +2
==========================================
+ Hits 1317 1325 +8
Misses 37 37 ☔ View full report in Codecov by Sentry. |
[skip ci] ## [3.6.0](v3.5.0...v3.6.0) (2024-07-08) ### Features * add hostnameExceptionList for ssrf ([#100](#100)) ([92a34f3](92a34f3))
Summary by CodeRabbit
New Features
hostnameExceptionList
for specifying allowed hostnames within IP blacklists, enhancing SSRF protection.Bug Fixes
refererWhiteList
to improve security configuration handling.Tests
hostnameExceptionList
feature.Documentation
hostnameExceptionList
in security configurations.