-
-
Notifications
You must be signed in to change notification settings - Fork 275
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
fix: add sldev.cz domain into whitelist to allow for testing of on Suite upgrade flow #16895
Conversation
WalkthroughThe change modifies the application’s domain configuration by adding a new entry to the 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (8)
🪧 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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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
🧹 Nitpick comments (1)
packages/suite-desktop-core/src/config.ts (1)
16-16
: Consider documenting VPN requirements more prominently.While the inline comment is helpful, consider documenting the VPN requirements and test environment details more prominently in the project's documentation to ensure all developers are aware of the access requirements.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/suite-desktop-core/src/config.ts
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: run-desktop-tests (@group=wallet, trezor-user-env-unix bitcoin-regtest)
- GitHub Check: run-desktop-tests (@group=other, trezor-user-env-unix)
- GitHub Check: run-desktop-tests (@group=settings, trezor-user-env-unix bitcoin-regtest)
- GitHub Check: run-desktop-tests (@group=device-management, trezor-user-env-unix)
- GitHub Check: run-desktop-tests (@group=suite, trezor-user-env-unix)
- GitHub Check: Setup and Cache Dependencies
- GitHub Check: build-web
- GitHub Check: Analyze with CodeQL (javascript)
🔇 Additional comments (1)
packages/suite-desktop-core/src/config.ts (1)
16-16
: Verify VPN access restrictions for the test domain.The comment indicates this is a VPN-protected test environment. Please ensure:
- The VPN requirement is properly enforced
- The domain is not accessible from public networks
Let's verify the domain's accessibility:
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Check if the domain is accessible without VPN # Expected: Connection timeout or refused, indicating VPN requirement curl -I --connect-timeout 5 https://sldev.czLength of output: 326
Domain VPN Restriction Confirmed
The output shows that attempting to access https://sldev.cz without VPN results in a DNS resolution error ("Could not resolve host: sldev.cz"), which confirms the domain is not publicly accessible. This behavior aligns with the intended security configuration for a VPN-protected test environment.
• Verify that the DNS/VPN configuration is correctly set up so that the domain becomes resolvable only when connected to the VPN.
• Consider enhancing internal documentation to more prominently state the VPN dependency for accessing this test domain.
5d18877
to
91e4d65
Compare
QA OK tested on macARM /it will be retested on other platforms and in any case of problem it will be reported/ Info:
|
No description provided.