Skip to content

fix(security): remediate findings from security assessment#351

Merged
remyluslosius merged 38 commits into
mainfrom
fix/security-assessment-remediation
Apr 14, 2026
Merged

fix(security): remediate findings from security assessment#351
remyluslosius merged 38 commits into
mainfrom
fix/security-assessment-remediation

Conversation

@remyluslosius
Copy link
Copy Markdown
Contributor

Summary

Implements code fixes for 14 findings identified in docs/OW_SECURITY_ASSESSMENT.md. These fixes correspond to the specs and tests added in the previous PR.

Critical (1)

  • C-1: Registration privilege escalation — enforce GUEST role for unauthenticated registration

High (2)

  • H-5: SQL injection in bulk_scan_orchestrator.py — parameterized IN clauses
  • H-6: Unsafe tar.extractall() in Kensa updater — path traversal validation

Medium (8)

  • M-1: Unauthenticated /metrics endpoint — added get_current_user dependency
  • M-3: Missing RBAC on webhook endpoints — added @require_role to all 7 endpoints
  • M-7: Rate limiter HMAC secret regenerated per-request — now initialized once in __init__
  • M-8: decode_token hardcoded "api_key" role — now resolves from database, falls back to GUEST
  • M-10: Health check leaks error details — removed str(e) from unhealthy response
  • M-12: Hardcoded paramiko.AutoAddPolicy() in 2 locations — replaced with SSHConfigManager.configure_ssh_client()
  • M-13: Unsafe zip.extractall() in marketplace and development plugin services — path traversal validation
  • M-14: Unsanitized filename in plugin upload — sanitize with Path.name and strip ..

Low (1)

  • L-2: Hardcoded "127.0.0.1" in register/logout audit logs — replaced with get_client_ip()

Informational (1)

  • I-3: Encryption service lacks AAD support — added optional aad parameter to encrypt()/decrypt()

New Functionality

  • PUT /api/admin/security/config/mfa — system-wide MFA enforcement toggle (SUPER_ADMIN only)
  • GET /api/admin/security/config/mfa — get current MFA enforcement setting

Files Changed (14)

File Finding
auth.py M-8: API key role resolution
encryption/service.py I-3: AAD support
main.py M-1: Metrics auth, M-10: Health check
middleware/rate_limiting.py M-7: HMAC secret init
plugins/kensa/updater.py H-6: Tar path traversal
routes/admin/security.py AC-15: MFA enforcement endpoints
routes/auth/login.py C-1: Registration role, L-2: Audit IPs
routes/hosts/crud.py M-12: AutoAddPolicy
routes/integrations/webhooks.py M-3: Webhook RBAC
routes/plugins/updates.py M-14: Filename sanitization
services/bulk_scan_orchestrator.py H-5: SQL injection
services/plugins/development/service.py M-13: Zip extractall
services/plugins/marketplace/service.py M-13: Zip extractall
services/ssh/connection_manager.py M-12: AutoAddPolicy

Test plan

  • 118/118 locally-runnable spec tests pass
  • All pre-commit hooks pass (black, isort, flake8, mypy, bandit, detect-secrets)
  • 403/403 spec ACs covered (check-spec-coverage.py)
  • CI pipeline passes
  • Docker integration tests confirm MFA endpoint and metrics auth work end-to-end

Loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants