Fix security vulnerabilities and resolve issues #131, #113, #108 #132
+26,914
−238
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Replaced all `Promise<any>` with proper `VyOSResponse` types in access-list API - Created shared `VyOSResponse` and `ApiError` interfaces in `lib/types/api.ts` - Removed unused `parseError` variable in API client - Improved type definitions for batch operations - **Pydantic V2 Migration:** - Updated deprecated `Field(example=...)` to `json_schema_extra` in dummy.py and ethernet.py - Migrated `class Config` to `model_config = ConfigDict()` in interface routers - Added `ConfigDict` imports where needed - **Backend Bug Fixes:** - Fixed async/await issue in `get_vyos_timezone()` function in power.py - Updated test client from deprecated httpx.AsyncClient to FastAPI TestClient - **Security and Production Readiness:** - Updated .env.example files for HTTPS production deployment - Added HTTPS configuration examples for both frontend and backend - **Code Quality:** - Removed unused variables and improved error handling - Updated deprecated Pydantic syntax for better compatibility This commit addresses critical code review issues including type safety, deprecated API usage, and production security configurations. All changes maintain backward compatibility and do not break existing functionality.
Co-authored-by: xTITUSMAXIMUSX <[email protected]>
Co-authored-by: xTITUSMAXIMUSX <[email protected]>
, Community-VyProjects#113, Community-VyProjects#108 Security fixes: - Add API key encryption using Fernet symmetric encryption - Remove dev fallback for auth secret, require 32+ character secret - Add rate limiting to authentication endpoints (slowapi) - Add CSRF protection middleware with Origin/Referer validation - Add security headers middleware (X-Frame-Options, CSP, HSTS, etc.) - Fix session fixation vulnerability with token mismatch detection - Replace detailed error messages with generic responses - Fix CORS headers - replace wildcard with explicit whitelist - Add method whitelist for dynamic firewall operations - Replace secrets.choice with uuid.uuid4 for ID generation Bug fixes: - Community-VyProjects#131: Fix frontend restart loop - add build check before npm start - Community-VyProjects#113: Improve DHCP leases parsing with regex for robust VyOS format handling - Community-VyProjects#108: Fix SSL verification - clear service cache when instance settings change Code quality: - Add structured logging (replace print statements) - Add audit logging utility for tracking user actions - Fix global mutable state in config snapshots (use app.state) - Improve CSV import validation (length limits, format checks, encryption) New files: - backend/utils/crypto.py - API key encryption utilities - backend/utils/logging.py - structured logging utilities - backend/utils/audit.py - audit logging for database - backend/middleware/csrf.py - CSRF protection middleware New dependencies: - cryptography>=41.0.0 - slowapi>=0.1.9
Co-Authored-By: Claude Opus 4.5 <[email protected]>
Jarnster
added
bug
Contributor
Author
|
Hi guys!Did you checked fixes? |
Member
Hello! First of all, thank you very much for your work! This is a large PR which involves a hefty amount of changes. This means that reviewing this takes a bit more time than usual. We need to make sure this PR doesn't break anything else. Multiple maintainers will need to review and agree before merging. We've tried deploying your PR already, but encountered several issues regarding session handling. Can you reverify how this could have broke? |
- Support both secure and non-secure cookie names (__Secure- prefix) - Remove strict token validation causing random disconnections - Fix API key fingerprint race condition in VyOS service cache - Update frontend proxy for secure cookie support Resolves production session issues reported by maintainers
Contributor
Author
|
Hi guys, I've identified and fixed issues. The root cause was that the code wasn't handling Better-Auth's production-specific behavior: What broke
Fixed in 4 files
Now it's working |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
npm startin productionSecurity Fixes (Code Review Findings):
secrets.choicewithuuid.uuid4()for session IDsNew Files:
backend/utils/crypto.py- API key encryption utilitiesbackend/utils/logging.py- Structured logging with security eventsbackend/utils/audit.py- Audit logging for database eventsbackend/middleware/csrf.py- CSRF protection middlewareDependencies Added:
cryptography>=41.0.0- For Fernet encryptionslowapi>=0.1.9- For rate limiting