pplate/cannamanage
1
0
Fork 0
Code Issues 9 Pull Requests Actions Packages Projects Releases Wiki Activity

docs: Sprint 5 security review (Snyk + SonarQube)

Browse Source
This commit is contained in:
Patrick Plate
2026-06-12 21:00:03 +02:00
parent 4d64576f22
commit 5c02cb0cde
1 changed files with 264 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/sprint-5/cannamanage-sprint5-security-review.md
+264
View File
@@ -0,0 +1,264 @@
# Security Review: Sprint 5
**Date:** 2026-06-12
**Scope:** Full codebase (backend + frontend) after Sprint 5
**Tools:** Snyk Code (SAST), Snyk Open Source (SCA), SonarQube (SAST via MCP)
---
## Summary
| Scan | Critical | High | Medium | Low | Status |
|------|----------|------|--------|-----|--------|
| Snyk Code (Frontend) | 0 | 0 | 2 | 4 | ⚠️ Low risk |
| Snyk Code (Backend) | 0 | 1 | 0 | 2 | ⚠️ Review needed |
| Snyk SCA (Frontend) | 0 | 5 | 14 | 0 | ⚠️ Upgrade needed |
| Snyk SCA (Backend) | 0 | 0 | 0 | 0 | ✅ Clean |
| SonarQube (Security) | 0 | 0 | 0 | 0 | ✅ Clean |
**Overall Verdict:** ⚠️ No critical blockers. 1 High-severity SAST finding (CSRF — false positive for stateless JWT API). 5 High-severity SCA findings in transitive dependencies (upgrade `next` to fix 2, remaining 3 in eslint tooling). No secrets exposure, no injection vectors, no auth bypass paths.
---
## Findings
### HIGH Severity
#### 1. Snyk Code: CSRF Protection Disabled — `SecurityConfig.java:47`
**Finding:** `.csrf(csrf -> csrf.disable())` on the `/api/**` filter chain.
**Verdict: ✅ FALSE POSITIVE — Intentional design.**
This API uses stateless JWT authentication with `SessionCreationPolicy.STATELESS`. CSRF protection is irrelevant for stateless token-based APIs because:
- No cookies carry authentication state (Bearer tokens in Authorization header)
- The browser never automatically attaches credentials
- CSRF attacks rely on cookie-based session state
The portal filter chain (`/portal/**`) correctly enables CSRF via `CookieCsrfTokenRepository`. The public endpoints filter chain also disables CSRF — acceptable for Swagger/actuator.
**Action:** None required. Add a comment explaining the rationale.
---
#### 2. Snyk SCA: Allocation of Resources Without Limits — `next@15.5.18`
**Vulnerability:** [SNYK-JS-NEXT-15105315](https://security.snyk.io/vuln/SNYK-JS-NEXT-15105315) (High)
**Fix:** Upgrade `next` to `>=16.1.7`
**Action:** Evaluate Next.js 16 upgrade feasibility. Pin to latest 15.x patch if 16 is breaking.
---
#### 3. Snyk SCA: ReDoS in `ajv@6.12.6`
**Vulnerability:** [SNYK-JS-AJV-15274295](https://security.snyk.io/vuln/SNYK-JS-AJV-15274295) (High)
**Path:** `@eslint/eslintrc@3.2.0 > ajv@6.12.6`
**Fix:** `ajv` ≥ 6.14.0
**Action:** Dev dependency only — not in production bundle. Low real-world risk.
---
#### 4. Snyk SCA: Infinite loop in `brace-expansion@1.1.12`
**Vulnerability:** [SNYK-JS-BRACEEXPANSION-15789759](https://security.snyk.io/vuln/SNYK-JS-BRACEEXPANSION-15789759) (High)
**Path:** `@eslint/eslintrc@3.2.0 > minimatch@3.1.2 > brace-expansion@1.1.12`
**Fix:** `brace-expansion` ≥ 1.1.13
**Action:** Dev dependency only (eslint toolchain). Override in `package.json` resolutions.
---
#### 5. Snyk SCA: ReDoS in `minimatch@3.1.2`
**Vulnerability:** [SNYK-JS-MINIMATCH-15309438](https://security.snyk.io/vuln/SNYK-JS-MINIMATCH-15309438) (High)
**Path:** `@eslint/eslintrc@3.2.0 > minimatch@3.1.2`
**Fix:** `minimatch` ≥ 3.1.3
**Action:** Dev dependency only. Override in resolutions.
---
#### 6. Snyk SCA: Inefficient Algorithmic Complexity in `minimatch@3.1.2`
**Vulnerability:** [SNYK-JS-MINIMATCH-15353389](https://security.snyk.io/vuln/SNYK-JS-MINIMATCH-15353389) (High)
**Path:** `@eslint/eslintrc@3.2.0 > minimatch@3.1.2`
**Fix:** `minimatch` ≥ 3.1.3
**Action:** Same as above — override in resolutions.
---
#### 7. Snyk SCA: ReDoS in `picomatch@4.0.3`
**Vulnerability:** [SNYK-JS-PICOMATCH-15765511](https://security.snyk.io/vuln/SNYK-JS-PICOMATCH-15765511) (High)
**Path:** `next-intl@4.13.0 > @parcel/watcher@2.5.6 > picomatch@4.0.3`
**Fix:** `picomatch` ≥ 4.0.4
**Action:** Production dependency via `next-intl`. Override `picomatch` to `4.0.4` in `pnpm.overrides`.
---
### MEDIUM Severity
#### 8. Snyk Code: Hardcoded Password — `src/data/user.ts:8`
**Finding:** `password: "StrongPass123"` in mock user data.
**Verdict: ⚠️ LOW RISK — Placeholder mock data from UI template (Shadboard starter kit).**
This file is only used for the sidebar user display component. It does NOT authenticate against the backend. However, shipping mock credentials in production bundles is poor practice.
**Action:** Remove the `password` field from `userData` — it is not consumed anywhere in the UI.
---
#### 9. Snyk Code: HTTP Cleartext — `e2e/mock-backend.mjs:16`
**Finding:** `http.createServer()` uses unencrypted HTTP.
**Verdict: ✅ FALSE POSITIVE — Test-only file.**
This is a local E2E test mock server used by Playwright. Never deployed.
**Action:** None required.
---
#### 10. Snyk SCA: Prototype Pollution in `picomatch@4.0.3`
**Vulnerability:** [SNYK-JS-PICOMATCH-15765513](https://security.snyk.io/vuln/SNYK-JS-PICOMATCH-15765513) (Medium)
**Fix:** `picomatch` ≥ 4.0.4
**Action:** Same override as finding #7.
---
#### 11. Snyk SCA: XSS in `postcss@8.4.31`
**Vulnerability:** [SNYK-JS-POSTCSS-16189065](https://security.snyk.io/vuln/SNYK-JS-POSTCSS-16189065) (Medium)
**Path:** `next@15.5.18 > postcss@8.4.31`
**Fix:** `postcss` ≥ 8.5.10
**Action:** Override `postcss` in `pnpm.overrides`, or wait for Next.js upgrade to bundle newer postcss.
---
#### 12. Snyk SCA: 10× LGPL-3.0 License Issues — `@img/sharp-libvips-*@1.2.4`
**Finding:** Sharp image library bundles LGPL-licensed libvips binaries across platform variants.
**Verdict: ⚠️ LICENSE COMPLIANCE — Not a security issue.**
LGPL-3.0 allows dynamic linking without copyleft contamination. Sharp uses these as pre-built native binaries (dynamic linking). No source disclosure obligation for the application.
**Action:** Document license acceptance in project records. No code change needed.
---
### LOW Severity
#### 13. Snyk Code: Hardcoded Passwords in Tests
| File | Line | Context |
|------|------|---------|
| `cannamanage-api/src/test/java/.../AuthControllerIntegrationTest.java` | 48 | Test credential |
| `cannamanage-api/src/test/java/.../AuthIntegrationTest.java` | 24 | Test secret |
| `cannamanage-frontend/e2e/authenticated-tour.spec.ts` | 152 | E2E test credential |
**Verdict: ✅ FALSE POSITIVE — Test-only scope.** These are hardcoded test credentials in test files. They don't match production passwords and are not deployed.
**Action:** None required. Standard testing practice.
---
#### 14. Snyk Code: Missing Secure Cookie Attribute
| File | Line | Context |
|------|------|---------|
| `src/components/ui/sidebar.tsx` | 93 | UI preference cookie |
| `e2e/authenticated-tour.spec.ts` | 52 | E2E test cookie setup |
| `e2e/screenshot-tour.spec.ts` | 50 | E2E test cookie setup |
**Verdict: ⚠️ MINOR for sidebar.tsx — the cookie stores UI sidebar state (open/closed), not auth data.**
**Action:** Add `secure: true` to the sidebar cookie in production builds (good practice, even for non-sensitive data). E2E test files are acceptable — localhost doesn't support Secure flag.
---
## SonarQube Analysis (MCP)
| File | Issues | Security Findings |
|------|--------|-------------------|
| `SecurityConfig.java` | 4 maintainability (duplicate literals) | 0 security |
| `AuthService.java` | 1 maintainability (generic exception in SHA-256 catch) | 0 security |
| `JwtAuthFilter.java` | 0 (excl. path false positive) | 0 security |
| `auth.ts` (NextAuth config) | 0 | 0 security |
**SonarQube Verdict: ✅ No security issues detected.** The code follows secure patterns:
- Proper token validation before extraction
- Token revocation check (JTI blacklist)
- TenantContext cleanup in `finally` block
- Generic error messages in auth (no info leakage)
- SHA-256 for refresh token storage (appropriate for high-entropy tokens)
- Proper refresh token rotation on each use
---
## False Positives Summary
| # | Tool | File | Finding | Reason |
|---|------|------|---------|--------|
| 1 | Snyk Code | `SecurityConfig.java:47` | CSRF disabled | Stateless JWT API — CSRF irrelevant |
| 2 | Snyk Code | `e2e/mock-backend.mjs:16` | HTTP cleartext | Test-only mock server |
| 3 | Snyk Code | `*IntegrationTest.java` | Hardcoded passwords | Test-scope only |
| 4 | Snyk Code | `e2e/*.spec.ts` | Missing Secure cookie | E2E test setup on localhost |
| 5 | Snyk SCA | `ajv`, `minimatch`, `brace-expansion` | High-severity ReDoS | Dev-only eslint toolchain deps |
---
## Recommendations
### Immediate (Sprint 6)
1. **Remove `password` field from `src/data/user.ts`** — unused mock data that triggers SAST scanners
2. **Add `pnpm.overrides`** for `picomatch@4.0.4` and `postcss@8.5.10` — fixes production-path vulnerabilities
3. **Add `secure: true`** to sidebar cookie in `sidebar.tsx` (line 93) when `window.location.protocol === 'https:'`
### Near-term (Sprint 7-8)
4. **Evaluate Next.js 16 upgrade** — fixes 2 High-severity `next` vulnerabilities (resource throttling)
5. **Upgrade `@eslint/eslintrc`** or add `pnpm.overrides` for `minimatch@3.1.3` and `ajv@6.14.0` — cleans dev toolchain
### Architecture Notes (Already Good)
- ✅ JWT auth with refresh token rotation and revocation
- ✅ Portal uses session-based auth with CSRF protection (separate filter chain)
- ✅ Password hashing via BCrypt
- ✅ Generic error messages in auth flow (no user enumeration)
- ✅ Invite token flow with expiry and single-use guarantee
- ✅ TenantContext isolation per request with `finally` cleanup
- ✅ Token blacklist via JTI for immediate revocation
- ✅ CORS restricted to known origins
- ✅ Method-level security via `@PreAuthorize`
---
## Appendix: Scan Commands
```bash
# Snyk Code (SAST) — Frontend
cd cannamanage-frontend && snyk code test
# Snyk Code (SAST) — Backend (full repo)
cd cannamanage && snyk code test
# Snyk SCA — Frontend
cd cannamanage-frontend && snyk test --all-projects
# Snyk SCA — Backend
cd cannamanage && snyk test --file=pom.xml
# SonarQube — via MCP analyze_code_snippet on key security files
```