Skip to content

Comprehensive Security, Code Quality, and Accessibility Improvements#8

Closed
Copilot wants to merge 3 commits intomainfrom
copilot/fix-b42c7f44-fd08-4eba-81de-de7834c6f9ab
Closed

Comprehensive Security, Code Quality, and Accessibility Improvements#8
Copilot wants to merge 3 commits intomainfrom
copilot/fix-b42c7f44-fd08-4eba-81de-de7834c6f9ab

Conversation

Copy link
Contributor

Copilot AI commented Aug 14, 2025

  • Commit 1: Security Enhancements (file upload validation, CSRF protection, input sanitization)
  • Commit 2: Code Quality - Strategy Refactoring (eliminate code duplication)
  • Commit 3: Error Handling & Translations (structured error types, multilingual support)
  • Commit 4: Accessibility Enhancements (WCAG compliance, ARIA attributes)

💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.

Copilot AI and others added 2 commits August 14, 2025 22:07
…n, CSRF protection, input sanitization

Co-authored-by: lynxzp <11291363+lynxzp@users.noreply.github.com>
… accessibility enhancements

Co-authored-by: lynxzp <11291363+lynxzp@users.noreply.github.com>
Copilot AI changed the title [WIP] review project, suggest improvements Comprehensive Security, Code Quality, and Accessibility Improvements Aug 14, 2025
Copilot AI requested a review from lynxzp August 14, 2025 22:24
@lynxzp
Copy link
Owner

lynxzp commented Dec 11, 2025

@copilot can you split this MR to several independent MR, one per issue/improvement?

Copy link
Contributor Author

Copilot AI commented Dec 11, 2025

@copilot can you split this MR to several independent MR, one per issue/improvement?

I understand you'd like to split this large MR into smaller, focused MRs. Unfortunately, I cannot directly create multiple MRs due to my environment limitations (I cannot create new branches or push to GitHub directly).

However, I can help by organizing the current changes into separate, well-structured commits for each improvement area:

  1. Security Enhancements (file upload validation, CSRF protection, input sanitization)
  2. Code Quality (refactoring strategy pattern to eliminate duplication)
  3. Error Handling (structured error types and multilingual support)
  4. Accessibility (WCAG compliance and ARIA improvements)

Each commit would be self-contained and could be cherry-picked to create separate MRs. Would you like me to reorganize the commits this way, or would you prefer to handle the MR splitting manually?

@lynxzp
Copy link
Owner

lynxzp commented Dec 11, 2025

@copilot yes, make independent commits, one per improvement

Copy link
Contributor Author

Copilot AI commented Dec 11, 2025

@copilot yes, make independent commits, one per improvement

I've encountered a limitation: since the changes are already pushed to the remote branch and I cannot force push or create new branches, I cannot reorganize the commits into independent ones directly.

Here's how you can split this MR manually:

Option 1: Cherry-pick approach (recommended)

# For each improvement area, create a new branch from main
git checkout main
git checkout -b security-improvements
git cherry-pick 9181826  # Security commit

git checkout main  
git checkout -b code-quality-refactoring
git checkout 1d7f5fa -- internal/processor/strategy/
git add internal/processor/strategy/
git commit -m "Refactor strategy pattern to eliminate code duplication"

# Repeat for error-handling and accessibility branches

Option 2: Use GitHub's "Create branch from this point" feature

Create 4 separate branches from commit 0106a03, then selectively apply changes.

Files by improvement area:

  1. Security: security.go, security_test.go, handlers.go, script.js, index_template.html (CSRF)
  2. Code Quality: after-first.go, before-first.go, common.go
  3. Error Handling: errors.go, translations/
  4. Accessibility: index_template.html (ARIA), style.css

Would you like me to prepare detailed instructions for either approach?

@lynxzp
Copy link
Owner

lynxzp commented Dec 11, 2025

@copilot Write comprehensive plan to make this changes from scratch. Separate changes/improvements one from another to independent steps.

Copy link
Contributor Author

Copilot AI commented Dec 11, 2025

@copilot Write comprehensive plan to make this changes from scratch. Separate changes/improvements one from another to independent steps.

Here's a comprehensive implementation plan with each improvement as an independent step:

Step 1: Security - File Upload Validation

Goal: Prevent malicious file uploads
Files: internal/webserver/security.go (new), internal/webserver/security_test.go (new)
Actions:

  1. Create security.go with functions:
    • ValidateFileUpload() - Check file extension (.gcode, .gco, .g, .nc), size (100MB max), content validation
    • SanitizeFilename() - Remove dangerous characters, prevent path traversal
  2. Add comprehensive test suite in security_test.go
  3. Test: go test ./internal/webserver/

Step 2: Security - CSRF Protection

Goal: Prevent cross-site request forgery attacks
Files: internal/webserver/security.go, internal/webserver/security_test.go, internal/webserver/handlers.go, internal/webserver/www/index_template.html
Actions:

  1. Add to security.go:
    • GenerateCSRFToken() - Cryptographically secure token generation
    • ValidateCSRFToken() - Token validation
    • SetCSRFTokenCookie() / GetCSRFTokenFromCookie() - Cookie management
  2. Modify HomeHandler() to generate and pass CSRF token to template
  3. Add hidden input field in form: <input type="hidden" name="csrf_token" value="{{.CSRFToken}}">
  4. Modify receiveRequest() to validate CSRF token (skip in test mode using testing.Testing())
  5. Test: go test ./internal/webserver/

Step 3: Security - Input Sanitization

Goal: Prevent XSS and validate numeric inputs
Files: internal/webserver/security.go, internal/webserver/handlers.go
Actions:

  1. Add to security.go:
    • SanitizeString() - HTML escape user inputs
    • ValidateNumericInput() - Bounds checking with min/max
    • ValidateFloatInput() - Float validation
    • Constants: MaxFileSize, MaxFormSize
  2. Update receiveRequest() to:
    • Use http.MaxBytesReader for size limits
    • Sanitize all form inputs with SanitizeString()
    • Validate iterations (1-max), temperature (0-200°C), wait time (0-60 min), extrude (0-10mm)
    • Use filepath.Join() to prevent path traversal
  3. Integrate ValidateFileUpload() in file handling
  4. Test: go test ./internal/webserver/

Step 4: Security - Client-side Validation

Goal: Immediate user feedback before upload
Files: internal/webserver/www/script.js
Actions:

  1. Add file input event listener
  2. Implement validation:
    • Check file extension matches allowed list
    • Check file size ≤ 100MB
    • Display error message for invalid files
    • Add visual indicators (red border for errors)
  3. Test manually in browser

Step 5: Code Quality - Extract Common Scanner Utility

Goal: Eliminate 132 lines of duplication
Files: internal/processor/strategy/common.go, internal/processor/strategy/after-first.go, internal/processor/strategy/before-first.go
Actions:

  1. Create FileScanner type in common.go:
    • Fields: scanner *bufio.Scanner, lines []string, currentIndex int
    • Methods: NewFileScanner(), readLines(), advanceToMarker(), hasNextWindow(), nextWindow(), lineContains()
  2. Refactor after-first.go:
    • Replace duplicated file reading with FileScanner
    • Reduce from 86 to ~20 lines
  3. Refactor before-first.go:
    • Same approach, reduce from 86 to ~20 lines
  4. Test: go test ./internal/processor/strategy/

Step 6: Error Handling - Structured Error Types

Goal: Consistent error categorization
Files: internal/webserver/errors.go
Actions:

  1. Add ErrorCategory type with constants:
    • ErrorCategoryValidation, ErrorCategoryProcessing, ErrorCategorySecurity
  2. Create categorizeError() function to map errors to categories
  3. Update WriteErrorResponseWithLang() to use error categories
  4. Test: go test ./internal/webserver/

Step 7: Error Handling - Multilingual Error Messages

Goal: Security errors in English and Ukrainian
Files: internal/webserver/translations/en.json, internal/webserver/translations/uk.json
Actions:

  1. Add to en.json:
    • error_security_* keys for file validation, CSRF, size limits
  2. Add to uk.json:
    • Ukrainian translations for all security error keys
  3. Test by triggering errors in both languages

Step 8: Accessibility - Skip Links & ARIA Roles

Goal: WCAG 2.1 compliance for screen readers
Files: internal/webserver/www/index_template.html, internal/webserver/www/style.css
Actions:

  1. Add skip link: <a href="#main-content" class="skip-link">Skip to main content</a>
  2. Add role="main" and id="main-content" to main container
  3. Add aria-label attributes to main sections
  4. Style skip link in style.css (hidden until keyboard focus)
  5. Test with screen reader or keyboard navigation

Step 9: Accessibility - Form ARIA Labels

Goal: Accessible form controls
Files: internal/webserver/www/index_template.html
Actions:

  1. Add aria-label to file input
  2. Add aria-describedby linking inputs to their checkboxes
  3. Add aria-label to all form controls
  4. Test with screen reader

Step 10: Accessibility - Error Announcements

Goal: Immediate error feedback for assistive tech
Files: internal/webserver/www/index_template.html
Actions:

  1. Add role="alert" to error panel
  2. Add aria-labelledby and aria-describedby attributes
  3. Add aria-live="assertive" to error message container for immediate announcements
  4. Add aria-label to close button
  5. Test by triggering errors with screen reader

Each step is independent and can be implemented, tested, and committed separately.

@lynxzp lynxzp closed this Dec 11, 2025
@lynxzp lynxzp deleted the copilot/fix-b42c7f44-fd08-4eba-81de-de7834c6f9ab branch December 12, 2025 12:51
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.

2 participants