fix(security): complete SSRF remediation with defense-in-depth (CWE-918) #436
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
feat: add API-Friendly security header preset for mobile apps
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
- Add constant-time token comparison utility (crypto/subtle) - Add SBOM generation and attestation to CI/CD pipeline - Document TLS enforcement, DNS security (DoH/DoT), and container hardening - Create Security Incident Response Plan (SIRP) - Add security update notification documentation Security enhancements: - Mitigates timing attacks on invite token validation - Provides supply chain transparency with CycloneDX SBOM - Documents production container hardening (read_only, cap_drop) Closes #365
…urity docs: add planning document for Issue #365 Additional Security
Contributor
There was a problem hiding this comment.
Pull request overview
This PR implements security enhancements from Issue #365, focusing on timing attack prevention, supply chain security, and comprehensive security documentation. The changes include constant-time token comparison utilities, SBOM generation in CI/CD, and extensive documentation for TLS security, DNS hijacking protection, container hardening, and incident response procedures.
Key Changes:
- Added constant-time comparison utilities to prevent timing attacks on sensitive token operations
- Implemented SBOM generation and attestation in the Docker build workflow for supply chain transparency
- Created comprehensive Security Incident Response Plan (SIRP) documentation
- Enhanced security documentation covering TLS, DNS, and container hardening best practices
Reviewed changes
Copilot reviewed 15 out of 16 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
backend/internal/util/crypto.go |
New constant-time comparison utilities for secure token validation |
backend/internal/util/crypto_test.go |
Comprehensive tests and benchmarks for crypto utilities |
backend/internal/api/handlers/user_handler.go |
Applied constant-time comparison to invite token verification |
docs/security.md |
Added TLS security, DNS security, and container hardening sections |
docs/security-incident-response.md |
New comprehensive incident response plan documentation |
docs/getting-started.md |
Added security update notification guidance |
.github/workflows/docker-build.yml |
Added SBOM generation and attestation steps |
docs/reports/qa_report.md |
Updated QA report for Issue #365 testing results |
docs/plans/issue-365-additional-security.md |
Planning document for security enhancements |
docs/plans/current_spec.md |
Implementation specification with codebase analysis |
docs/issues/issue-365-manual-test-plan.md |
Manual testing procedures for security features |
Comments suppressed due to low confidence (1)
docs/security-incident-response.md:401
- The file ends with a closing markdown code fence that should be removed. This closing fence corresponds to the unnecessary opening fence at line 1.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
6 tasks
…xed header
Enhance the Layout component with two critical UI/UX improvements:
1. Scrollable Sidebar Navigation:
- Add overflow-y-auto to navigation area between logo and logout
- Apply flex-shrink-0 to logout section to keep it anchored at bottom
- Add min-h-0 to enable proper flexbox shrinking
- Prevents logout button from being pushed off-screen when multiple
submenus are expanded
- Custom scrollbar styling for both light and dark themes
2. Fixed Header Bar:
- Change desktop header from relative to sticky positioning
- Header remains visible at top when scrolling main content
- Move overflow control from main container to content wrapper
- Proper z-index hierarchy maintained (header z-10, sidebar z-30)
- Mobile header behavior unchanged (already fixed)
Technical Details:
- Modified Layout.tsx: 7 targeted CSS class changes
- Modified index.css: Added WebKit and Firefox scrollbar styling
- CSS-only implementation (no JavaScript overhead)
- Hardware-accelerated scrolling for optimal performance
Testing:
- Frontend coverage: 87.59% (exceeds 85% threshold)
- Backend coverage: 86.2% (regression tested)
- Zero security vulnerabilities (Trivy scan)
- No accessibility regressions
- Cross-browser tested (Chrome, Firefox, Safari)
Breaking Changes: None
Backward Compatibility: Full
Files Changed:
- frontend/src/components/Layout.tsx
- frontend/src/index.css
Documentation:
- Updated CHANGELOG.md with UI enhancements
- Created comprehensive implementation summary
- Created detailed QA reports and manual test plan
…dation and preview functionality
Add configurable public-facing URL setting to fix issue where invite emails contained internal localhost addresses inaccessible to external users. Features: - New "Application URL" setting in System Settings (key: app.public_url) - Real-time URL validation with visual feedback and HTTP warnings - Test button to verify URL accessibility - Invite preview showing actual link before sending - Warning alerts when URL not configured - Fallback to request-derived URL for backward compatibility - Complete i18n support (EN, DE, ES, FR, ZH) Backend: - Created utils.GetPublicURL() for centralized URL management - Added POST /settings/validate-url endpoint - Added POST /users/preview-invite-url endpoint - Updated InviteUser() to use configured public URL Frontend: - New Application URL card in SystemSettings with validation - URL preview in InviteModal with warning banners - Test URL button and configuration warnings - Updated API clients with validation and preview functions Security: - Admin-only access for all endpoints - Input validation prevents path injection - SSRF-safe (URL only used in email generation) - OWASP Top 10 compliant Coverage: Backend 87.6%, Frontend 86.5% (both exceed 85% threshold) Refs: #application-url-feature
- Make COOP header conditional on development mode to suppress HTTP warnings - Add autocomplete attributes to all email/password inputs for password manager compatibility - Add comprehensive tests for COOP conditional behavior - Update security documentation for COOP, HTTPS requirements, and autocomplete Fixes browser console warnings and improves UX by enabling password managers. All quality gates passed: 85.7% backend coverage, 86.46% frontend coverage, zero security issues, all pre-commit hooks passed. Changes: - Backend: backend/internal/api/middleware/security.go - Frontend: Login, Setup, Account, AcceptInvite, SMTPSettings pages - Tests: Added 4 new test cases (2 backend, 2 frontend) - Docs: Updated security.md, getting-started.md, README.md
…mproved warning accuracy
…n clarity in TestURLConnectivity
…stReconcileCrowdSecOnStartup
…or improved SSRF protection clarity
…tDisabled for clarity
…te URL validation comments
…ves and update workflow to use new config
fix(security): complete SSRF remediation with defense-in-depth (CWE-918)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
Implements additional security enhancements as outlined in Issue #365.
Security Threats Addressed
High Priority
Medium Priority
Documentation Updates
Future Considerations (Out of Scope)
Related Issues
Closes #365
Checklist