From fb7cd1d40d5521937d6bf6092a47ccfb033fb9de Mon Sep 17 00:00:00 2001 From: Freddy Ouma <103921342+oumafreddy@users.noreply.github.com> Date: Tue, 31 Mar 2026 21:39:26 +0300 Subject: [PATCH 1/3] Expand security review with cookie security and privacy guidance --- SECURITY_REVIEW_2026-03-31.txt | 327 +++++++++++++++++++++++++++++++++ 1 file changed, 327 insertions(+) create mode 100644 SECURITY_REVIEW_2026-03-31.txt diff --git a/SECURITY_REVIEW_2026-03-31.txt b/SECURITY_REVIEW_2026-03-31.txt new file mode 100644 index 0000000..56deef7 --- /dev/null +++ b/SECURITY_REVIEW_2026-03-31.txt @@ -0,0 +1,327 @@ +# Oreno Security Review (Attack-Surface Oriented) + +**Date:** 2026-03-31 +**Reviewer:** GPT-5.3-Codex +**Method:** Static code/config review of repository contents (no deployed environment validation) + +--- + +## 1) Executive Summary + +This review analyzed the Oreno repository by attack surface: **Web application**, **APIs**, **Cloud/infrastructure**, and **CI/CD pipelines**, plus explicit notes for **mobile** and **gRPC/WebSocket/GraphQL** scope. + +### Overall risk posture + +- The project shows strong baseline controls in multiple areas: role-aware permissions, tenant-aware middleware, CSRF usage in templates, and existing security tests. +- However, there are several **high-impact design gaps** that can expose sensitive functions or weaken tenant isolation assumptions when specific endpoints are hit. + +### Highest-priority findings + +1. **Public AI assistant API endpoint with `permission_classes = []` in core** (unauthenticated access). +2. **Agent executor update/delete paths do not apply explicit org-level object scoping before mutation** (depends on schema/tenant assumptions). +3. **Webhook egress can call arbitrary URLs without anti-SSRF allowlist/blocklist controls.** +4. **Organization middleware broadly exempts `/api/` paths from org-context enforcement**, creating inconsistent trust boundaries for API code paths. +5. **CSP/script policy remains permissive (`unsafe-inline`, `unsafe-eval`) and duplicates CSP logic in middleware/settings**, making hardening brittle. + +--- + +## 2) Scope and Method + +## In-scope surfaces reviewed + +- **Web application** (authn/authz, input handling, business logic, client-side/security headers) +- **APIs** (REST and internal agent execution endpoints) +- **Cloud/infrastructure config signals** (Django/ASGI/security settings) +- **CI/CD pipeline** (GitHub Actions workflow) + +## Out of practical scope (repo evidence only) + +- Live cloud account posture (IAM, buckets, metadata exposure) not directly testable from repo alone. +- Mobile client binaries/source were not found in repo. +- No active GraphQL/gRPC endpoint definitions found. + +--- + +## 3) Findings by Attack Surface + +## A) Web Application Attack Surface + +### A1. Unauthenticated AI endpoint in `apps/core/views.py` (High) + +- `AIAssistantAPIView` sets `permission_classes = []`, and accepts POST data directly. +- This endpoint is separate from the authenticated AI service endpoints and can be reached without login depending on routing and middleware behavior. + +**Evidence:** `apps/core/views.py`. + +**Risk:** Unauthenticated abuse (prompt abuse, resource burn, data leakage if model backend uses sensitive context defaults). + +**Recommendation:** Require `IsAuthenticated` (or signed anonymous session with strict rate limits + CAPTCHA + no sensitive context). + +--- + +### A2. Organization middleware API exemption widens trust boundary (High) + +- `OrganizationMiddleware.EXEMPT_PATH_PREFIXES` includes `/api/` and then explicitly clears org context (`request.organization = None`). +- This means API handlers cannot rely on this middleware for uniform org binding and must each re-implement tenant/org checks correctly. + +**Evidence:** `apps/core/middleware.py`. + +**Risk:** Inconsistent authorization controls and accidental cross-org data access in endpoints that assume middleware-scoped org context. + +**Recommendation:** Remove blanket `/api/` exemption and use explicit endpoint-level exemptions only where needed. + +--- + +### A3. Notification APIs intentionally allow unauthenticated access (Medium) + +- Notification list API classes use `permission_classes = []` and return empty arrays for anonymous callers. + +**Evidence:** `apps/audit/api_views.py`, `apps/audit/views.py`. + +**Risk:** Low direct data exposure now, but creates a pattern that can regress into leakage if future code adds data before auth checks. + +**Recommendation:** Use explicit `IsAuthenticated`; return empty arrays only after auth in business logic if required by frontend UX. + +--- + +### A4. CSP hardening incomplete + duplicate CSP providers (Medium) + +- CSP in settings allows `'unsafe-inline'` and `'unsafe-eval'` in places. +- Separate CSP header logic is also injected in `common.middleware.CSPNonceMiddleware`, potentially conflicting with `django-csp` configuration. + +**Evidence:** `config/settings/base.py`, `common/middleware.py`. + +**Risk:** XSS impact amplification, policy drift, and unclear source of truth for header behavior. + +**Recommendation:** Consolidate CSP generation to one mechanism (prefer framework config), remove unsafe directives where feasible, and deploy with nonce/hash-only scripts progressively. + +--- + +### A5. File upload controls rely on extension/size but virus scanning validator is not enforced here (Medium) + +- Document uploads validate extension and size. +- A virus-scanning validator exists but is not attached to Document model fields in reviewed paths. + +**Evidence:** `apps/document_management/models.py`, `apps/core/models/validators.py`. + +**Risk:** Malicious file upload acceptance where downstream consumers may execute/open risky content. + +**Recommendation:** Attach `validate_file_virus` to upload fields or enforce async scanning + quarantine before availability. + +--- + +### A6. Cookie security and privacy posture is partially hardened, but consent/state cookies are weak and tracking consent flow is unclear (High) + +- Server-side session and CSRF controls are mostly good in production (`Secure`, `HttpOnly` for session, `SameSite=Lax`), but CSRF is intentionally not `HttpOnly` in active settings paths. +- The frontend consent cookie (`cookies_accepted=true`) is set via JavaScript without `Secure`, `SameSite`, or explicit domain constraints. +- Cookie policy page embeds TikTok identify logic hashing user identifiers client-side; this can still be personal data processing and should be consent-gated. + +**Evidence:** `config/settings/base.py`, `config/settings/production.py`, `config/settings/development.py`, `static/js/cookies.js`, `templates/public/cookies.html`. + +**Risk:** +1) Consent cookie can be manipulated/cross-context leaked more easily without explicit attributes. +2) Inconsistent consent enforcement may trigger privacy non-compliance risk if tracking calls execute before valid consent. +3) CSRF cookie script-access is common but increases exposure impact if XSS exists. + +**Recommendation:** +- Set consent cookie with `Secure; SameSite=Lax` (or `Strict` where possible) and consider server-set cookie flags. +- Implement explicit consent categories and gate analytics/ads scripts (including TikTok identify) behind opt-in state. +- Keep a consent event log/versioning approach to prove policy acceptance state over time. +- Re-evaluate `CSRF_COOKIE_HTTPONLY=False` tradeoff and retain only if frontend architecture requires script access. + +--- + +## B) API Attack Surface (REST / Agent) + +### B1. Agent executor mutation paths are not object-scoped by organization before update/delete (High) + +- In `AgentExecutor`, `_update` and `_delete` fetch by `id` without additional org filter in those methods. +- `_read` has explicit organization filtering, but mutation paths rely on tenant context and serializer behavior. + +**Evidence:** `services/agent/executor.py`. + +**Risk:** If tenant schema assumptions are bypassed/misconfigured or shared schema objects exist, this becomes IDOR-style mutation risk. + +**Recommendation:** Enforce org filter in all object lookups for update/delete (`.get(id=obj_id, organization_id=request.user.organization_id)` where applicable) and reject if model has organization and org is missing. + +--- + +### B2. AI async API uses `permission_classes = []` with decorator protection (Low/Medium) + +- `AIAssistantAsyncAPIView` has empty DRF permissions but relies on `@login_required` dispatch override. + +**Evidence:** `services/ai/views.py`. + +**Risk:** Mixed auth paradigms increase maintenance risk; future refactors may break decorator coverage. + +**Recommendation:** Add DRF-native `permission_classes = [IsAuthenticated]` and keep decorators only for HTML views. + +--- + +### B3. Registration serializer includes sensitive fields via inherited user serializer (Medium) + +- `UserRegisterSerializer` extends `UserSerializer` field set including `role`, `is_staff`, `organization`, etc.; no explicit write restriction besides model defaults/permissions. + +**Evidence:** `apps/users/serializers.py`, `apps/users/views.py`. + +**Risk:** Privilege/tenant assignment abuse if endpoint protections are changed or misapplied. + +**Recommendation:** Define a minimal explicit registration serializer field list and hardcode server-side defaults for role/staff/org assignment. + +--- + +## C) SSRF / Egress Attack Surface + +### C1. Webhook service can POST to arbitrary URLs without private-network protections (High) + +- Webhook URLs are stored and posted to directly with `requests.post(...)`. +- No hostname allowlist, IP resolution checks, protocol restrictions beyond URLField format, or metadata IP deny rules are visible. + +**Evidence:** `apps/ai_governance/models.py`, `apps/ai_governance/webhook_service.py`, `apps/ai_governance/serializers.py`, `apps/ai_governance/forms.py`. + +**Risk:** SSRF toward internal services/cloud metadata; pivot to internal network reconnaissance/exfiltration. + +**Recommendation:** Implement strict webhook URL validation: +- enforce `https` only, +- block localhost/private/link-local/metadata ranges, +- optional org-level allowlist, +- DNS rebinding defenses (resolve + verify each request), +- egress proxy/policy with deny-by-default. + +--- + +## D) Client-Side Attack Surface + +### D1. CSP currently permits script execution patterns commonly abused by XSS (Medium) + +- `'unsafe-inline'` and `'unsafe-eval'` are still present in script policies (at least in some code paths). + +**Evidence:** `config/settings/base.py`, `common/middleware.py`. + +**Risk:** Any HTML/DOM injection vulnerability becomes easier to weaponize. + +**Recommendation:** remove unsafe directives incrementally, especially in production. + +--- + +## E) Business Logic / Multi-Tenant Isolation + +### E1. Multi-layer tenant/org enforcement is present but inconsistent across API and middleware paths (Medium/High) + +- There are strong controls (`OrganizationScopedQuerysetMixin`, role permissions, tenant middleware), but blanket API exemptions in org middleware force endpoint-by-endpoint correctness. + +**Evidence:** `apps/core/middleware.py`, `apps/core/mixins/organization.py`, multiple viewsets in apps. + +**Risk:** Future endpoint additions may omit org filtering and become cross-tenant leaks. + +**Recommendation:** centralize and require one enforcement primitive for all APIs; add regression tests for every new ViewSet action. + +--- + +## F) Infrastructure / Cloud Signals + +### F1. Production settings generally hardened, but cookie/CSRF and proxy trust should be reviewed in deployment context (Low/Medium) + +- Positive controls: secure cookies, HSTS, SSL redirect, nosniff, XFO, Redis/cache usage. +- `CSRF_COOKIE_HTTPONLY=False` (often acceptable for JS patterns, but broadens token exposure to XSS). + +**Evidence:** `config/settings/production.py`. + +**Recommendation:** Keep as-is if required by frontend architecture, otherwise consider HTTPOnly CSRF pattern alternatives and stricter same-site/token handling. + +--- + +## G) CI/CD Pipeline Attack Surface + +### G1. Action pinning is not commit-SHA strict; some actions are version tags or `@master` (Medium) + +- Workflow uses tags (`@v4`, `@v3`) and one floating branch (`aquasecurity/trivy-action@master`). + +**Evidence:** `.github/workflows/ci.yml`. + +**Risk:** Supply-chain compromise through action hijack/tag move. + +**Recommendation:** pin all third-party actions to immutable commit SHAs and enable Dependabot updates. + +--- + +### G2. Security scanners run with `|| true` (Low/Medium) + +- `bandit` and `safety` do not fail pipeline on findings. + +**Evidence:** `.github/workflows/ci.yml`. + +**Risk:** Known vulnerabilities do not block merges. + +**Recommendation:** fail on thresholded severity; keep artifact upload for triage. + +--- + +## 4) API Modality Coverage Notes + +- **REST APIs:** Extensive and reviewed. +- **WebSockets:** Channels scaffold exists but no active ws routes configured (`websocket_urlpatterns` empty). +- **GraphQL:** Not found. +- **gRPC:** Not found. + +**Evidence:** `config/asgi.py`. + +--- + +## 5) Mobile Application Surface + +No Android/iOS app code was found in this repository. Mobile-specific risks (local storage, reverse engineering, cert pinning) are therefore **not assessed here**. + +--- + +## 6) Prioritized Remediation Plan (30/60/90) + +## First 30 days (blockers) + +1. Lock down unauthenticated AI endpoint in `apps/core/views.py`. +2. Add explicit org filters for agent update/delete/read/create operations where model contains organization. +3. Deploy SSRF-safe webhook URL validation + private-network block + HTTPS-only egress. +4. Remove `/api/` blanket org-check exemption in middleware; replace with precise exemptions. +5. Harden cookie handling: secure consent cookie attributes and enforce explicit opt-in gating for analytics/ads identifiers. + +## Next 60 days + +6. Consolidate CSP handling and remove unsafe directives in production. +7. Enforce malware scanning/quarantine for all file uploads. +8. Pin GitHub Actions to immutable SHAs and make security scans enforceable. + +## Next 90 days + +9. Add automated “authz matrix” tests for every API route (anonymous/user/manager/admin/cross-org). +10. Add threat-model docs for tenant boundary assumptions (schema-per-tenant and org object ownership). +11. Add central secure egress service for all outbound HTTP integrations. + +--- + +## 7) Suggested Verification Tests + +1. Anonymous POST to `/api/ai/ask/` and any core AI endpoint; confirm 401/403 where intended. +2. Agent execute update/delete using object IDs from other org contexts; confirm hard deny. +3. Webhook registration attempts with `http://127.0.0.1`, `http://169.254.169.254`, private RFC1918 IPs; confirm validation failure. +4. CSP header snapshot tests (single authoritative header, no unsafe directives in production). +5. CI policy test that fails if an action is unpinned or a scanner step is non-blocking for high severity. +6. Consent/cookie test: verify non-essential trackers do not run until consent is explicitly granted, and consent cookie carries `Secure` + `SameSite`. + +--- + +## 8) Security Strengths Observed + +- Good use of role-based permission classes and tenant-aware mixins in many API modules. +- Presence of dedicated security tests (`tests/security/*`). +- Production settings include core hardening controls (HSTS, secure cookies, SSL redirect). +- JWT blacklist and OTP/first-time setup controls are present. + +--- + +## 9) Final Notes + +This review is repository-static and should be paired with: +- dynamic testing in a staging deployment, +- authenticated attack-path testing per role, +- cloud account posture review (IAM, buckets, metadata controls), +- and CI/CD runner hardening review. From c5e3e555d9943f6fb20597fa0380fd8c67741404 Mon Sep 17 00:00:00 2001 From: Freddy Ouma <103921342+oumafreddy@users.noreply.github.com> Date: Tue, 31 Mar 2026 21:50:49 +0300 Subject: [PATCH 2/3] Fix false-positive attack detection on business text inputs --- SECURITY_REVIEW_2026-03-31.txt | 4 ++-- common/middleware.py | 23 +++++++++++++++++++---- common/tests/test_middleware.py | 26 +++++++++++++++++++++++++- 3 files changed, 46 insertions(+), 7 deletions(-) diff --git a/SECURITY_REVIEW_2026-03-31.txt b/SECURITY_REVIEW_2026-03-31.txt index 56deef7..94c001b 100644 --- a/SECURITY_REVIEW_2026-03-31.txt +++ b/SECURITY_REVIEW_2026-03-31.txt @@ -114,7 +114,7 @@ This review analyzed the Oreno repository by attack surface: **Web application** - Server-side session and CSRF controls are mostly good in production (`Secure`, `HttpOnly` for session, `SameSite=Lax`), but CSRF is intentionally not `HttpOnly` in active settings paths. - The frontend consent cookie (`cookies_accepted=true`) is set via JavaScript without `Secure`, `SameSite`, or explicit domain constraints. -- Cookie policy page embeds TikTok identify logic hashing user identifiers client-side; this can still be personal data processing and should be consent-gated. +- Cookie policy page includes third-party tracking identify logic with hashed user identifiers; this can still be personal data processing and should be consent-gated. **Evidence:** `config/settings/base.py`, `config/settings/production.py`, `config/settings/development.py`, `static/js/cookies.js`, `templates/public/cookies.html`. @@ -125,7 +125,7 @@ This review analyzed the Oreno repository by attack surface: **Web application** **Recommendation:** - Set consent cookie with `Secure; SameSite=Lax` (or `Strict` where possible) and consider server-set cookie flags. -- Implement explicit consent categories and gate analytics/ads scripts (including TikTok identify) behind opt-in state. +- Implement explicit consent categories and gate analytics/ads scripts behind opt-in state. - Keep a consent event log/versioning approach to prove policy acceptance state over time. - Re-evaluate `CSRF_COOKIE_HTTPONLY=False` tradeoff and retain only if frontend architecture requires script access. diff --git a/common/middleware.py b/common/middleware.py index 5c0f21a..20c1b63 100644 --- a/common/middleware.py +++ b/common/middleware.py @@ -115,7 +115,7 @@ class SecurityMiddleware: """ logger = logging.getLogger('security.middleware') - # SQL injection patterns + # SQL injection patterns (keep high-signal only to reduce false positives) SQL_INJECTION_PATTERNS = [ r'(?i)(union.*select|select.*from|insert.*into|delete.*from|update.*set|drop.*table)', r'(?i)(or\s+\d+\s*=\s*\d+|and\s+\d+\s*=\s*\d+)', @@ -126,7 +126,6 @@ class SecurityMiddleware: r'(?i)(information_schema|sys\.|pg_catalog)', r'(?i)(char\s*\(|chr\s*\(|concat|cast\s*\()', r'(?i)(--\s*$|/\*|\*/|#\s*$)', - r'(?i)(\'\s*(or|and)\s*\'|\"\s*(or|and)\s*\")', ] # XSS patterns @@ -235,10 +234,26 @@ def _contains_attack_patterns(self, request): return True # Check POST data (if any) + # NOTE: + # We intentionally use a stricter subset for POST body inspection to avoid + # false positives on normal business text fields (e.g., action plans with + # natural language like "and", "or", hyphens, etc.). if request.method == 'POST' and request.POST: post_data = str(request.POST).lower() - for pattern in self.SQL_INJECTION_PATTERNS + self.XSS_PATTERNS: + post_patterns = [ + # SQLi high confidence + r'(?i)(union.*select|select.*from|insert.*into|delete.*from|update.*set|drop.*table)', + r'(?i)(sleep\s*\(|waitfor\s+delay|pg_sleep|benchmark\s*\()', + r'(?i)(exec\s*\(|execute\s*\(|sp_executesql)', + r'(?i)(xp_cmdshell|xp_regread|xp_dirtree)', + r'(?i)(information_schema|sys\.|pg_catalog)', + # XSS high confidence + r'(?i)( Date: Tue, 31 Mar 2026 23:07:34 +0300 Subject: [PATCH 3/3] Use CSRF cookie for HTMX headers and remove token meta tag --- static/js/main.js | 13 +++++++++++-- templates/base.html | 1 - 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/static/js/main.js b/static/js/main.js index 937500c..e02d749 100644 --- a/static/js/main.js +++ b/static/js/main.js @@ -13,7 +13,7 @@ }); function initCSRFHandling() { - const csrfToken = document.querySelector('meta[name="csrf-token"]')?.getAttribute('content'); + const csrfToken = getCookie('csrftoken'); if (csrfToken) { // Set up CSRF token for all AJAX requests document.addEventListener('htmx:configRequest', function(evt) { @@ -24,6 +24,15 @@ } } + function getCookie(name) { + const value = `; ${document.cookie}`; + const parts = value.split(`; ${name}=`); + if (parts.length === 2) { + return parts.pop().split(';').shift(); + } + return null; + } + function initUIComponents() { // Check if Bootstrap is available if (typeof bootstrap !== 'undefined' && typeof UIUtils !== 'undefined') { @@ -339,4 +348,4 @@ if (typeof module !== 'undefined' && module.exports) { showToast: uiUtils.showToast, sanitizeInput }; -} \ No newline at end of file +} diff --git a/templates/base.html b/templates/base.html index eae0bbe..5e4d125 100644 --- a/templates/base.html +++ b/templates/base.html @@ -12,7 +12,6 @@ -