diff --git a/.opencode/agent/repo-auditor.md b/.opencode/agent/repo-auditor.md index 3da001e..d2e46a4 100644 --- a/.opencode/agent/repo-auditor.md +++ b/.opencode/agent/repo-auditor.md @@ -22,7 +22,7 @@ You audit the **entire SplitIt repository** and report on code quality, architec ## Method -1. **Map the repo.** Read `AGENTS.md` and `docs/specs/` first — they are the source of truth. Then read the prior reports (`docs/PRODUCTION_AUDIT.md`, `docs/SECURITY.md`, `docs/REMEDIATION_REPORT_PHASE_0.5.md`) so you don't re-report already-remediated findings — verify them instead. Inventory `SplitIt.API/` (API, Application, Domain, Infrastructure, Shared) and `split-it-ui/src/app`. +1. **Map the repo.** Read `AGENTS.md` and `docs/specs/` first — they are the source of truth. Then read the prior reports (`docs/PRODUCTION_AUDIT.md`, `docs/SECURITY.md`) so you don't re-report already-remediated findings — verify them instead. Inventory `SplitIt.API/` (API, Application, Domain, Infrastructure, Shared) and `split-it-ui/src/app`. 2. **Load the relevant skills** before judging an area: `dotnet-best-practices`, `aspnet-core`, `csharp-async`, `dotnet-design-pattern-review`, `api-contract`, `angular-best-practices`, `security-review`, `data-integrity-audit`, `i18n`, `accessibility`, `db-migrations`. 3. **Read the real code** — controllers, services, entities, `AppDbContext`, migrations, Angular components/services/guards/interceptors, specs, configs, Dockerfiles and CI. Don't judge from file names or from the docs alone. 4. **Verify, don't guess.** Run `npm run build`, `npm run test` (or per side) when useful, and report the actual result. If you can't run something, say so. diff --git a/.opencode/skills/containerize-aspnetcore/SKILL.md b/.opencode/skills/containerize-aspnetcore/SKILL.md index 0a683e2..bda8c72 100644 --- a/.opencode/skills/containerize-aspnetcore/SKILL.md +++ b/.opencode/skills/containerize-aspnetcore/SKILL.md @@ -110,8 +110,7 @@ Any settings that are not specified will be set to default values. The default v ## Execution Process 1. Review the containerization settings above to understand the containerization requirements -2. Create a `progress.md` file to track changes with check marks -3. Determine the .NET version from the project's .csproj file by checking the `TargetFramework` element +2. Determine the .NET version from the project's .csproj file by checking the `TargetFramework` element 4. Select the appropriate Linux container image based on: - The .NET version detected from the project - The Linux distribution specified in containerization settings (Alpine, Ubuntu, Chiseled, or Azure Linux (Mariner)) @@ -175,36 +174,11 @@ docker build -t aspnetcore-app:latest . If the build fails, review the error messages and make necessary adjustments to the Dockerfile or project configuration. Report success/failure. -## Progress Tracking - -Maintain a `progress.md` file with the following structure: -```markdown -# Containerization Progress - -## Environment Detection -- [ ] .NET version detection (version: ___) -- [ ] Linux distribution selection (distribution: ___) - -## Configuration Changes -- [ ] Application configuration verification for environment variable support -- [ ] NuGet package source configuration (if applicable) - -## Containerization -- [ ] Dockerfile creation -- [ ] .dockerignore file creation -- [ ] Build stage created with SDK image -- [ ] csproj file(s) copied for package restore -- [ ] NuGet.config copied if applicable -- [ ] Runtime stage created with runtime image -- [ ] Non-root user configuration -- [ ] Dependency handling (system packages, native libraries, tools, etc.) -- [ ] Health check configuration (if applicable) -- [ ] Special requirements implementation - -## Verification -- [ ] Review containerization settings and make sure that all requirements are met -- [ ] Docker build success -``` +## Reporting + +Do not create progress or report files. When the work is done, reply with a short summary: +environment detected, files created, and the actual `docker build` result. Durable decisions belong +in the project's docs (`AGENTS.md`, `docs/specs/`, ADRs) — never in a per-task `progress.md`. Do not pause for confirmation between steps. Continue methodically until the application has been containerized and Docker build succeeds. diff --git a/AGENTS.md b/AGENTS.md index 0a91633..44490b4 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -20,7 +20,7 @@ SplitIt/ ├─ SplitIt.Tests/ # Backend tests (referenced from SplitIt.API/SplitIt.Back.sln) ├─ split-it-ui/ # Angular application (src/app, e2e) ├─ docker/ # Docker configs (backend, frontend, proxy, sqlserver) -├─ docs/ # Guides, reports, screenshots, specs (docs/specs/) +├─ docs/ # Guides, runbooks, specs, ADRs (docs/specs/, docs/adr/) ├─ scripts/ # Deploy, helper and local-dev orchestration scripts (run-splitit.mjs) ├─ package.json # Root orchestration scripts (dev, db:*, docker:dev, build, test) ├─ .opencode/ # AI home: agent/, command/, skills/ (tracked; local plugin scaffold ignored) @@ -59,7 +59,19 @@ Angular 21 app in `split-it-ui/src/app`. Protected routes via JWT, admin panel b - Skills: `api-contract`, `i18n`, `db-migrations`, `backend-test`, `frontend-test`, `run-e2e`, `docker-dev`, `security-review`, `angular-best-practices`, `dotnet-best-practices`, `data-integrity-audit`, plus imported generic ones. - Two review modes: `/review ` → **reviewer** agent (per-change); `/review repo` → **repo-auditor** agent (whole-repo graded report, read-only). - `opencode.json` holds instructions, MCP servers and permissions. Skills, agents and commands need no config — opencode auto-discovers `.opencode/`. -- `AGENTS.md` is the single source of truth; `docs/specs/` holds details and `docs/AUDIT_*.md` holds audit reports. +- `AGENTS.md` is the single source of truth; `docs/specs/` holds details, `docs/adr/` holds decisions and `docs/AUDIT_*.md` holds the current audit. + +## Documentation Policy + +Docs capture decisions and current state, never session narration. + +- **Allowed:** `README` (how to run), `docs/adr/NNN-*.md` (one decision: context, options, decision, + consequences), `docs/specs/*.md` (current design and business rules), runbooks + (`DEPLOYMENT`, `BACKUPS`, `CICD`, `DOCKER`, `HTTPS`, `NGINX`, `TESTING`), and the current + `docs/AUDIT_*.md` / `docs/PRODUCTION_AUDIT.md` / `docs/SECURITY.md`. +- **Forbidden:** phase reports, progress logs, "what I did" narration and per-session summaries. + When a change needs a durable record, update the relevant spec or add an ADR — do not create a + report file. This applies to AI output too. ## Working Rules For This Repo - Language: all code, comments, XML docs, tests, commit messages, PR titles/descriptions, docs (`README`, `docs/`, `AGENTS.md`), and AI output must be in English. Only user-facing UI strings may be in Spanish (via i18n files), never hardcoded Spanish in code/comments. diff --git a/docs/AUDIT_2026-09.md b/docs/AUDIT_2026-09.md index d327005..37ed213 100644 --- a/docs/AUDIT_2026-09.md +++ b/docs/AUDIT_2026-09.md @@ -4,7 +4,7 @@ > **Alcance:** repo completo en `main` @ `d5c4939`, read-only > **Método:** `repo-auditor` + skills `security-review`, `data-integrity-audit`, `dotnet-best-practices`, `angular-best-practices` > **Contexto:** app a punto de usarse por usuarios reales (amigos). Foco: no perder/corromper data y cumplimiento Ley 1581. -> **Reportes previos verificados:** `docs/PRODUCTION_AUDIT.md`, `docs/SECURITY.md`, `docs/REMEDIATION_REPORT_PHASE_0.5.md` (los hallazgos ya remediados NO se re-reportan; este documento se enfoca en lo abierto). +> **Reportes previos verificados:** `docs/PRODUCTION_AUDIT.md`, `docs/SECURITY.md` (los hallazgos ya remediados NO se re-reportan; este documento se enfoca en lo abierto). --- diff --git a/docs/CICD.md b/docs/CICD.md index a78c84e..b3961cb 100644 --- a/docs/CICD.md +++ b/docs/CICD.md @@ -293,6 +293,5 @@ CI/CD does not include monitoring. Future phases may add: scripts/ └── deploy.sh # VPS deployment script docs/ -├── CICD.md # This file -└── PHASE_13_REPORT.md # Phase report +└── CICD.md # This file ``` diff --git a/docs/PHASE_11_REPORT.md b/docs/PHASE_11_REPORT.md deleted file mode 100644 index 0988a1e..0000000 --- a/docs/PHASE_11_REPORT.md +++ /dev/null @@ -1,132 +0,0 @@ -# Phase 11 Report — HTTPS, Nginx Reverse Proxy & Production Security Headers - -## Goal -Implement production HTTPS and reverse-proxy architecture without weakening the existing security model. - -## Changed files - -| File | Change | -|------|--------| -| `docker-compose.yml` | Added `proxy` and `certbot` services; removed `frontend` port publication; added TLS volumes; updated health checks | -| `docker/proxy/Dockerfile` | New production Nginx reverse-proxy image (non-root, alpine) | -| `docker/proxy/nginx.conf.template` | Templated Nginx config with routing, rate limits, TLS | -| `docker/proxy/entrypoint.sh` | Runtime config rendering + self-signed cert generation | -| `docker/proxy/snippets/*.conf` | Security headers, SSL params, trusted proxies | -| `docker/frontend/nginx.conf` | Simplified to pure static server; removed duplicated security headers | -| `SplitIt.API/SplitIt.API/Program.cs` | Added `ForwardedHeaders` middleware (private ranges only) | -| `SplitIt.Tests/CorsTests.cs` | New CORS fail-closed tests | -| `split-it-ui/e2e/fullstack/docker-https-fullstack.spec.ts` | New HTTPS E2E suite through Nginx | -| `split-it-ui/e2e/fullstack/docker-fullstack.spec.ts` | Updated to use HTTPS through Nginx | -| `.env.example` | Added `DOMAIN`, `TLS_MODE`, `ACME_EMAIL`, `PROXY_HTTP_PORT`, `PROXY_HTTPS_PORT` | -| `.gitignore` / `.dockerignore` | Added certificate and key exclusions | -| `scripts/generate-local-ssl.*` | Helper scripts for local TLS pre-generation | -| `docs/NGINX.md`, `docs/HTTPS.md`, `docs/PHASE_11_REPORT.md` | Documentation | - -## Test results - -| Suite | Result | -|-------|--------| -| Backend `dotnet test` | **92 passed, 0 failed** | -| Frontend `npm run build` (production) | **Success** | -| Angular unit tests (`npm test`) | **Skipped** — Chrome not available in this environment | -| Playwright full suite | **66 passed, 0 failed** | -| Docker Compose validation | **Valid** | - -### Playwright highlights -- HTTP → HTTPS redirect: pass -- ACME challenge path stays HTTP (no redirect): pass -- Health endpoints return `text/plain` (not `index.html`): pass -- Security headers present on all HTTPS responses: pass -- Registration, login, authenticated API, protected routes, groups, expenses, settlement, BOLA, logout: all pass - -## Docker verification - -### Exposed ports (host) -``` -80/tcp -> proxy:8080 -443/tcp -> proxy:8443 -``` - -### NOT exposed -- `1433/tcp` (SQL Server) -- `8080/tcp` (backend direct) -- `80/tcp` (frontend direct) - -### Networks -- `splitit-frontend-net` — bridge (proxy, frontend, backend) -- `splitit-backend-net` — bridge **internal: true** (backend, sqlserver, db-init, migrator) - -### Privileged containers -None. - -### Host network / Docker socket -Not used. - -## TLS verification - -- TLS 1.2/1.3 only; obsolete protocols disabled. -- HSTS header present. -- Self-signed profile works for local testing. -- Let's Encrypt profile ready for production (requires DNS + `certbot` run). - -## CORS verification - -- Backend remains fail-closed: no `AllowAnyOrigin()`. -- Production `.env.example` sets `CORS_ALLOWED_ORIGINS=https://splitit.yourdomain.com` only. -- New `CorsTests.cs` verifies allowed origin, blocked origin, and empty-origin fail-closed behavior. - -## Trivy vulnerability scan - -| Image | HIGH / CRITICAL | -|-------|-----------------| -| splitit-backend | **0** | -| splitit-frontend | **0** | -| splitit-proxy | **0** | - -## Security headers - -All active on HTTPS responses: -- Strict-Transport-Security -- X-Content-Type-Options -- X-Frame-Options -- Referrer-Policy -- Permissions-Policy -- Cross-Origin-Opener-Policy -- Cross-Origin-Resource-Policy -- Content-Security-Policy (compatible with Angular production build) - -## Secrets / certificate audit - -- `.env` is ignored by Git. -- No certificate, private key, or password files are tracked. -- No secrets are baked into Docker images. - -## Remaining risks / limitations - -1. **Rate limiting on localhost tests:** Because all requests from the host share the same Docker gateway IP, Nginx rate limits can be exhausted during rapid repeated E2E runs. Production (one IP per real client) is not affected. -2. **Self-signed certificate warning:** Local browsers/tests must accept or ignore the warning; this is expected and documented. -3. **Angular unit tests:** Could not be executed in this environment due to missing Chrome installation; this is an environment limitation, not a code regression. -4. **Let's Encrypt renewal:** Requires operator to configure cron or manual renewal workflow; documented in `docs/HTTPS.md`. -5. **CSP `style-src 'unsafe-inline'`:** Required for Angular Material/Bootstrap compatibility. The application does not use inline scripts. - -## Proposed commit message - -``` -feat: implement HTTPS reverse proxy and production security headers (Phase 11) - -- Add nginx:alpine reverse proxy as the only Internet-facing container -- Terminate TLS 1.2/1.3 with hardened cipher suite and HSTS -- Support Let's Encrypt (production) and self-signed (local testing) modes -- Route /api/* and /health/* to backend; serve Angular SPA catch-all -- Apply security headers including CSP compatible with Angular production build -- Implement nginx-level rate limiting for auth, API, and static traffic -- Add ForwardedHeaders middleware with strict KnownNetworks -- Add CORS fail-closed tests -- Add HTTPS full-stack E2E tests through Nginx -- Remove frontend port exposure; only proxy exposes 80/443 -- Document architecture in docs/NGINX.md and docs/HTTPS.md -``` - -## Next step - -Wait for explicit approval before committing or starting Phase 12. diff --git a/docs/PHASE_12_REPORT.md b/docs/PHASE_12_REPORT.md deleted file mode 100644 index 5b39fbc..0000000 --- a/docs/PHASE_12_REPORT.md +++ /dev/null @@ -1,145 +0,0 @@ -# Phase 12 Report — HTTPS: Let's Encrypt, 80→443, Renewal - -## Goal -Automate the Let's Encrypt certificate lifecycle (renewal + Nginx reload) and validate the 80→443 redirect and ACME challenge path that were implemented in Phase 11. - -## Scope (from master plan, `docs/PRODUCTION_AUDIT.md:370`) -``` -Phase 12 HTTPS — Let's Encrypt, 80→443, renewal -``` - -Phase 11 delivered the Nginx reverse proxy, self-signed cert generation, manual Let's Encrypt issuance, HTTP→HTTPS 301 redirect, ACME HTTP-01 challenge path, TLS hardening, and security headers. Phase 12 adds **automated certificate renewal** with zero-downtime Nginx reload. - -## Changed files - -| File | Change | -|------|--------| -| `docker/proxy/certbot-renew.sh` | **New.** Automated renewal loop script. Performs initial issuance if certs are missing, then loops `certbot renew` every 12h. Uses `--deploy-hook` to write a sentinel file on successful renewal. | -| `docker/proxy/entrypoint.sh` | Added background watcher (every 60s) that checks for the renewal sentinel file in the shared `certbot_www` volume and runs `nginx -s reload` when found. | -| `docker-compose.yml` | Added `certbot-renewer` service (profile: `letsencrypt`, image: `certbot/certbot:latest`, mounts `certbot-renew.sh` + `certbot_certs` + `certbot_www` volumes, depends on `proxy:healthy`, resource limits 0.25 CPU / 64 MB). Updated `certbot` service with `DOMAIN` and `ACME_EMAIL` env vars. | -| `.env.example` | Added `COMPOSE_PROFILES` (commented, for enabling the renewer) and `RENEWAL_INTERVAL` (default 12h). | -| `docs/HTTPS.md` | Documented automated renewal workflow, reload mechanism diagram, manual renewal alternative, and new environment variables. | -| `SplitIt.Tests/Phase12HttpsTests.cs` | **New.** 22 tests validating: certbot-renew.sh content, entrypoint reload watcher, docker-compose certbot-renewer service config, `.env.example` variables, nginx 80→443 redirect, ACME path, TLS hardening, HSTS, `.dockerignore` cert exclusions, HTTPS docs. | -| `docs/PHASE_12_REPORT.md` | This report. | - -## Test results - -| Suite | Result | -|-------|-------| -| Backend `dotnet test` (Phase 12 filter) | **22 passed, 0 failed** | -| Backend `dotnet test` (full suite) | **114 passed, 4 failed** (pre-existing — see below) | -| Docker Compose config (default) | **Valid** (exit 0) | -| Docker Compose config (`--profile letsencrypt`) | **Valid** — certbot-renewer service present with correct volumes/network/resources | -| Docker Compose config (`--profile certbot`) | **Valid** — manual certbot service present | - -### Pre-existing failures (not caused by Phase 12) -`CorsTests` (4 tests) fail with `JwtSettings:SecretKey is missing or too short`. The `WebApplicationFactory` sets the secret via `ConfigureAppConfiguration`, but `Program.cs:31` validates it during top-level statement execution before deferred configuration is applied. **This predates Phase 12** — no Phase 12 file touches `Program.cs`, `CorsTests.cs`, or any application code. Documented per project rules; not fixed. - -### Phase 12 test details -| Test | Validates | -|------|-----------| -| `CertbotRenewScript_Exists` | Script file present and non-empty | -| `CertbotRenewScript_ContainsRenewalLoop` | `certbot renew`, `while true`, `sleep` present | -| `CertbotRenewScript_ContainsDeployHookSentinel` | `--deploy-hook` and `.reload-trigger` sentinel | -| `CertbotRenewScript_ContainsInitialIssuance` | `certonly`, `--webroot`, `--non-interactive` for first-time | -| `Entrypoint_ContainsReloadWatcher` | `.reload-trigger` and `nginx -s reload` in entrypoint | -| `Entrypoint_WatcherRunsInBackground` | Subshell `) &` and `sleep 60` | -| `Compose_HasCertbotRenewerService` | Service exists in docker-compose.yml | -| `Compose_CertbotRenewerHasLetsencryptProfile` | Profile `letsencrypt` set | -| `Compose_CertbotRenewerHasCorrectVolumes` | `certbot_certs`, `certbot_www`, `certbot-renew.sh` | -| `Compose_CertbotRenewerHasResourceLimits` | CPU and memory limits set | -| `Compose_CertbotRenewerDependsOnProxy` | `depends_on: proxy: service_healthy` | -| `EnvExample_DocumentsComposeProfiles` | `COMPOSE_PROFILES` and `letsencrypt` | -| `EnvExample_DocumentsRenewalInterval` | `RENEWAL_INTERVAL` variable | -| `NginxTemplate_HasHttpToHttpsRedirect` | `return 301 https://` present | -| `NginxTemplate_HasAcmeChallengePath` | `/.well-known/acme-challenge` present | -| `NginxTemplate_AcmePathHasNoRedirect` | ACME location block does NOT contain redirect | -| `HttpsDocs_DocumentAutomatedRenewal` | `certbot-renewer`, `sentinel`, `Phase 12` | -| `HttpsDocs_DocumentManualRenewal` | Manual `certbot renew` and `nginx -s reload` | -| `HttpsDocs_DocumentReloadMechanism` | `Automated reload` and `deploy-hook` | -| `SslParams_DisableObsoleteProtocols` | TLS 1.2/1.3 only, no 1.0/1.1 | -| `SslParams_HasHstsInSecurityHeaders` | HSTS with `max-age=31536000` and `preload` | -| `DockerIgnore_BlocksCertificateFiles` | `*.pem`, `*.key`, `*.crt` excluded | - -## Docker verification - -### Exposed ports (host) -``` -80/tcp -> proxy:8080 (HTTP → 301 HTTPS redirect + ACME challenge) -443/tcp -> proxy:8443 (HTTPS application traffic) -``` - -### NOT exposed -- `1433/tcp` (SQL Server) -- `8080/tcp` (backend direct) -- `80/tcp` (frontend direct) - -### Profiles -| Profile | Service | Purpose | -|---------|---------|---------| -| `certbot` | `splitit-certbot` | Manual one-off certbot operations (issuance, delete) | -| `letsencrypt` | `splitit-certbot-renewer` | Automated renewal loop (every 12h) + sentinel reload | - -### Networks -- `splitit-frontend-net` — bridge (proxy, frontend, backend, certbot, certbot-renewer) -- `splitit-backend-net` — bridge **internal: true** (backend, sqlserver, db-init, migrator) - -### Privileged containers / Docker socket -None. - -### Resource limits (certbot-renewer) -- 0.25 CPU, 64 MB RAM. - -## Renewal mechanism - -``` -certbot-renewer proxy (nginx) - | | - |-- certbot renew |-- serving HTTPS traffic - |-- on success: |-- background watcher (60s loop) - | deploy-hook: | if /var/www/certbot/.reload-trigger: - | touch .reload-trigger | rm .reload-trigger - | | nginx -s reload (zero-downtime) - v v - certbot_www volume <--------- shared ---------> -``` - -1. `certbot-renewer` runs `certbot renew` every 12 hours. -2. On successful renewal, certbot's `--deploy-hook` writes `.reload-trigger` to the shared `certbot_www` volume. -3. The proxy entrypoint's background watcher detects the sentinel within 60s, removes it, and reloads Nginx. -4. Nginx picks up the new certificate without dropping connections. - -## Security implications - -1. **No new exposed ports**: The certbot-renewer is an internal container; it does not publish any host ports. It communicates only via shared Docker volumes. -2. **No secrets in images**: The `certbot-renew.sh` script is a shell script mounted read-only; no certificates or private keys are baked into any image. `.dockerignore` continues to block `*.pem`, `*.key`, `*.crt`. -3. **Certbot-renewer runs as root** (default for `certbot/certbot` image): This is the standard certbot image behavior. The renewer only writes to `/etc/letsencrypt` and `/var/www/certbot` (both Docker volumes), not to the host filesystem. Resource limits are set (0.25 CPU, 64 MB). -4. **No authentication changes**: Phase 12 does not modify JWT, CORS, auth guard, or any application security logic. -5. **80→443 redirect** (from Phase 11) verified: All non-ACME HTTP traffic receives a `301` permanent redirect to HTTPS. The ACME challenge path (`/.well-known/acme-challenge/`) stays on HTTP and is NOT redirected, allowing certbot to validate domain ownership. -6. **HSTS**: `max-age=31536000; includeSubDomains; preload` remains active on all HTTPS responses. -7. **TLS hardening**: TLS 1.2/1.3 only, forward-secret ciphers, OCSP stapling — unchanged from Phase 11. -8. **Profile isolation**: The certbot-renewer only starts when `COMPOSE_PROFILES=letsencrypt` is set or `--profile letsencrypt` is passed. In self-signed mode (default for local testing), the renewer does not start. - -## Remaining risks - -1. **Initial issuance still manual**: The first Let's Encrypt certificate must be obtained manually (the proxy requires a cert to start in `letsencrypt` mode, creating a chicken-and-egg problem). The `certbot-renew.sh` script attempts initial issuance if certs are missing, but this requires the proxy to already be running and serving the ACME challenge path. Documented in `docs/HTTPS.md`. -2. **Sentinel file permissions**: The certbot-renewer (root) creates the `.reload-trigger` file. The proxy (nginx user, UID 101) removes it. This works because the `/var/www/certbot` directory is owned by nginx (initialized from the proxy image), and Unix directory write permission allows the owner to delete files regardless of file ownership. If the volume is first created by the certbot container (e.g., certbot starts before proxy), the directory may be root-owned and the watcher cannot delete the sentinel — nginx would reload every 60s (harmless but noisy). Mitigated by `depends_on: proxy: service_healthy` ensuring proxy starts first. -3. **Renewal loop not verified against real Let's Encrypt**: The automated renewal was validated via Docker Compose config and script content tests, but not against the real Let's Encrypt staging API (requires a public DNS domain). The script follows standard certbot patterns. -4. **Pre-existing CorsTests failure** (4 tests): `WebApplicationFactory` + `Program.cs` JWT validation timing issue. Unrelated to Phase 12. -5. **No renewal failure alerting**: If certbot renewal fails, the renewer logs the error but no alert is sent. Monitoring/alerting is Phase 20+ scope. -6. **Let's Encrypt rate limits**: If the renewer loops too quickly or multiple instances run, rate limits may be hit. The 12h interval and single-instance design mitigate this. - -## Proposed commit message - -``` -feat: automate Let's Encrypt certificate renewal with zero-downtime Nginx reload (Phase 12) - -- Add certbot-renewer service (profile: letsencrypt) running certbot renew every 12h -- Add certbot-renew.sh with initial issuance fallback and --deploy-hook sentinel -- Add background reload watcher in proxy entrypoint (checks shared volume every 60s) -- Add COMPOSE_PROFILES and RENEWAL_INTERVAL to .env.example -- Document automated + manual renewal workflows in docs/HTTPS.md -- Add 22 Phase12HttpsTests validating renewal automation, 80→443 redirect, - ACME path, TLS hardening, HSTS, and certificate file exclusions -- Validate docker-compose config with letsencrypt and certbot profiles -``` diff --git a/docs/PHASE_13_REPORT.md b/docs/PHASE_13_REPORT.md deleted file mode 100644 index 723865b..0000000 --- a/docs/PHASE_13_REPORT.md +++ /dev/null @@ -1,251 +0,0 @@ -# Phase 13 Report — CI/CD Pipeline - -## Goal -Implement a secure production CI/CD pipeline for SplitIt using GitHub Actions, with automated testing, security scanning, and controlled deployment to VPS. - -## Scope (from master plan, `docs/PRODUCTION_AUDIT.md:370`) -``` -Phase 17 CI/CD — GitHub Actions, Docker build, deployment -``` - -Phase 13 delivers the complete CI/CD infrastructure: GitHub Actions workflows for continuous integration, Docker image building with Trivy security scanning, and automated deployment to production VPS via SSH. - -## Changed files - -| File | Change | -|------|--------| -| `.github/workflows/ci.yml` | **New.** CI workflow with 6 jobs: backend tests (SQL Server service), frontend build/test, Playwright E2E, security scan, Docker build + Trivy, compose validation. | -| `.github/workflows/deploy.yml` | **New.** Production deployment workflow. SSH into VPS, pull code, build containers, verify health. Supports manual skip-tests for emergency deploys. | -| `scripts/deploy.sh` | **New.** VPS deployment script with pull/build/up/status/rollback/logs/verify commands. Creates backups, validates config, waits for health checks. | -| `docs/CICD.md` | **New.** Complete CI/CD documentation: pipeline architecture, required secrets, VPS configuration, deployment flow, rollback procedures, security considerations. | -| `docs/PHASE_13_REPORT.md` | This report. | - -## Test results - -| Suite | Result | -|-------|-------| -| Backend `dotnet test` | **Existing tests unchanged** — 114 passed, 4 pre-existing CorsTests failures | -| Frontend build | **`npm run build`** — Production build succeeds | -| Frontend tests | **Existing tests unchanged** — 25 Karma specs passing | -| Docker Compose config (default) | **Valid** (exit 0) | -| Docker Compose config (`--profile letsencrypt`) | **Valid** | -| Docker Compose config (`--profile certbot`) | **Valid** | -| YAML syntax validation | **Valid** — both workflows pass yamllint | - -### Pre-existing failures (not caused by Phase 13) -`CorsTests` (4 tests) fail with `JwtSettings:SecretKey is missing or too short`. This predates Phase 13 — no Phase 13 file touches application code. Documented per project rules; not fixed. - -## CI/CD Architecture - -### Pipeline Flow - -``` -PR/Push to main - │ - ├─── Backend Tests (ubuntu, SQL Server container) - │ ├── dotnet restore - │ ├── dotnet build - │ └── dotnet test + coverage - │ - ├─── Frontend Build & Test (ubuntu, Node 20) - │ ├── npm ci - │ ├── ng build --production - │ └── karma test + coverage - │ - ├─── E2E Tests (needs: frontend) - │ ├── npm ci - │ ├── ng build --production - │ ├── playwright install chromium - │ └── playwright test - │ - ├─── Security Scan (needs: backend, frontend) - │ ├── Check for secrets in code - │ ├── Verify .env not committed - │ └── Validate .gitignore/.dockerignore - │ - ├─── Docker Build & Trivy (needs: backend, frontend) - │ ├── Build backend image - │ ├── Build frontend image - │ ├── Build proxy image - │ ├── Trivy scan backend (HIGH/CRITICAL = fail) - │ ├── Trivy scan frontend (HIGH/CRITICAL = fail) - │ └── Trivy scan proxy (HIGH/CRITICAL = fail) - │ - └─── Docker Compose Validate - ├── Validate default config - ├── Validate letsencrypt profile - └── Validate certbot profile - -Push to main (after CI passes) - │ - └─── Deploy to VPS - ├── Run tests (unless skipped) - ├── Setup SSH key - ├── SSH into VPS - ├── Create backup - ├── Pull code - ├── Validate config - ├── Build containers - ├── Start containers - ├── Wait for health (120s) - ├── Verify services - └── Cleanup old backups -``` - -### Security Controls - -| Control | Implementation | -|---------|----------------| -| No secrets in Git | `.env` in `.gitignore` and `.dockerignore` | -| No secrets in logs | GitHub Actions masks secrets automatically | -| SSH key auth only | Private key in GitHub secrets, public key on VPS | -| Non-root deploy | VPS user `deploy` with docker group | -| Vulnerability scanning | Trivy fails on HIGH/CRITICAL | -| Secrets detection | CI scans code for hardcoded secrets | -| Backup before deploy | Automatic backup to `/opt/splitit-backup-*` | -| Health verification | 120s timeout, service-by-service check | -| Rollback support | `deploy.sh rollback` restores from backup | -| Concurrency control | Only one deploy at a time | - -## Secrets Required - -### GitHub Repository Secrets - -| Secret | Purpose | Where Used | -|--------|---------|------------| -| `VPS_SSH_PRIVATE_KEY` | SSH private key for VPS | `deploy.yml` | -| `VPS_HOST` | VPS hostname/IP | `deploy.yml` | -| `VPS_USER` | SSH username | `deploy.yml` | - -### VPS Environment (NOT in GitHub) - -| Variable | Purpose | -|----------|---------| -| `DB_PASSWORD` | SQL Server SA password | -| `DB_APP_USER` | Application DB user | -| `DB_APP_PASSWORD` | Application DB password | -| `DB_MIGRATOR_USER` | Migration DB user | -| `DB_MIGRATOR_PASSWORD` | Migration DB password | -| `JWT_SECRET` | JWT signing key | -| `JWT_ISSUER` | JWT issuer | -| `JWT_AUDIENCE` | JWT audience | -| `CORS_ALLOWED_ORIGINS` | CORS allowed origins | -| `DOMAIN` | Production domain | -| `TLS_MODE` | TLS mode (letsencrypt) | -| `ACME_EMAIL` | Let's Encrypt email | - -## Deployment Behavior - -### Normal Deployment -1. CI tests pass on PR/push -2. Push to main triggers deploy workflow -3. Tests run again (unless skipped) -4. SSH into VPS -5. Backup current deployment -6. Pull latest code -7. Validate docker compose config -8. Build Docker images (no cache) -9. Start containers with `--remove-orphans` -10. Wait up to 120s for health checks -11. Verify all 5 services (db-init, migrator, backend, frontend, proxy) -12. Cleanup old backups (keep last 3) - -### Emergency Deployment -- Use workflow_dispatch with `skip_tests: true` -- Skips test suite for critical hotfixes -- All other steps remain the same - -### Failure Handling -- If health checks fail → deploy fails, container logs printed -- If service unhealthy → deploy fails -- Previous deployment remains running until new one passes validation -- No automatic rollback on partial failure (manual rollback available) - -## Rollback Procedure - -### Automated (Recommended) -```bash -ssh deploy@VPS -cd /opt/splitit -./scripts/deploy.sh rollback -``` - -### Manual -```bash -ssh deploy@VPS -cd /opt/splitit -docker compose down -rm -rf /opt/splitit -cp -r /opt/splitit-backup-YYYYMMDD-HHMMSS /opt/splitit -cd /opt/splitit -docker compose up -d -docker compose ps -``` - -### Git-based -```bash -ssh deploy@VPS -cd /opt/splitit -git log --oneline -10 # Find commit -git reset --hard -docker compose build --no-cache -docker compose up -d -``` - -## Security Findings - -### Implemented Controls -1. **No secrets in Git** — `.env` excluded via `.gitignore` and `.dockerignore` -2. **No secrets in logs** — GitHub Actions masks secrets automatically -3. **SSH key authentication** — No password authentication -4. **Non-root deployment** — Deploy user with minimal privileges -5. **Vulnerability scanning** — Trivy blocks HIGH/CRITICAL -6. **Secrets detection** — CI scans for hardcoded secrets -7. **Backup before deploy** — Automatic backup creation -8. **Health verification** — Comprehensive service health checks -9. **Rollback support** — Manual and automated rollback options - -### Known Limitations -1. **No automated rollback** — Failed deploys require manual intervention -2. **No deployment notifications** — No Slack/email alerts -3. **No canary/blue-green** — All-or-nothing deployment -4. **No DB backup** — Database backup is manual (outside scope) -5. **Trivy false positives** — Some base image CVEs may block pipeline - -## Remaining Risks - -1. **Single point of failure** — VPS is single server (no HA) -2. **No database backup automation** — SQL Server data not backed up by CI/CD -3. **Trivy false positives** — Base image CVEs may require exemptions -4. **SSH key rotation** — No automated key rotation -5. **Secrets on VPS** — Production secrets stored in plaintext `.env` -6. **No deployment approval** — Direct push to main deploys to production -7. **No smoke tests** — Health checks verify service running, not functionality -8. **Concurrency** — Manual concurrent deploys possible (workflow_dispatch) - -## Proposed commit message - -``` -feat: implement CI/CD pipeline with GitHub Actions and VPS deployment (Phase 13) - -- Add ci.yml workflow: backend tests, frontend build/test, Playwright E2E, - security scan, Docker build + Trivy, compose validation -- Add deploy.yml workflow: SSH deployment to VPS with health verification -- Add deploy.sh script: pull/build/up/status/rollback/logs/verify commands -- Add CICD.md documentation: pipeline architecture, secrets, VPS config -- Trivy fails on HIGH/CRITICAL container vulnerabilities -- No secrets exposed in logs or committed to repository -- Production secrets remain on VPS only -- Automatic backup before deployment with rollback support -``` - -## Verification - -- [x] YAML syntax valid (both workflows) -- [x] Docker compose config valid (all profiles) -- [x] No secrets in git diff -- [x] No business logic modified -- [x] No Docker networking changed -- [x] No authentication/CORS/HTTPS changed -- [x] Documentation complete -- [x] Rollback procedure documented diff --git a/docs/PHASE_8_REPORT.md b/docs/PHASE_8_REPORT.md deleted file mode 100644 index d7491e0..0000000 --- a/docs/PHASE_8_REPORT.md +++ /dev/null @@ -1,218 +0,0 @@ -# Phase 8 — Future Features + Business Logic Hardening — Report - -> **Date:** 2026-08-24 -> **Status:** Implemented, not yet committed (awaiting approval before Phase 9 Docker) -> **Previous:** Phase 7 43 E2E (25+18) + 39 backend (36+3 skipped) passing - ---- - -## 1. Changed Files - -``` -Modified: - SplitIt.API/SplitIt.API/Controllers/ExpensesController.cs:86 (partial payment swapping, GetRemainingDebt, absolute amount) - SplitIt.API/SplitIt.API/Controllers/GroupsController.cs:114 (PUT/DELETE role, remove, delete) - SplitIt.API/SplitIt.Infrastructure/Services/ExpensesService.cs:202 (GetRemainingDebtAsync, RegisterPayment partial logic, rounding) - SplitIt.API/SplitIt.Infrastructure/Services/GroupService.cs:105 (IsUserAdminOrCreator, UpdateMemberRole, RemoveMember, DeleteGroup) - SplitIt.API/SplitIt.Infrastructure/Services/UsersService.cs:23 (GetAllUsers, IsUserAdmin, UpdateUserRole) - SplitIt.Tests/SettlementCrossGroupTests.cs:1 (import) - split-it-ui/src/app/modules/dashboard/components/group-detail/group-detail.component.ts:146 (Math.abs, remainingDebt snackbar) - split-it-ui/src/app/modules/dashboard/components/split-method-dialog/split-method-dialog.component.ts:61 (equal cents distribution, fixed/percentage validation) - -Added: - SplitIt.API/SplitIt.API/Controllers/AdminController.cs:1 (GET /admin/users, PUT /admin/users/{id}/role, Role 1/2 check) - SplitIt.API/SplitIt.Application/DTOs/UpdateGroupMemberRoleDto.cs:1 (admin|member regex) - SplitIt.Tests/PartialPaymentTests.cs:1 (7 tests) - SplitIt.Tests/GroupAdminTests.cs:1 (9 tests) - SplitIt.Tests/AppAdminTests.cs:1 (6 tests) - SplitIt.Tests/SplitMethodTests.cs:1 (6 tests) - SplitIt.Tests/MonetaryPrecisionTests.cs:1 (5 tests) - split-it-ui/e2e/phase8/partial-payments.spec.ts:1 (5) - split-it-ui/e2e/phase8/split-methods.spec.ts:1 (4) - split-it-ui/e2e/phase8/group-admin.spec.ts:1 (5) - split-it-ui/e2e/phase8/app-admin.spec.ts:1 (4) -``` - -> **Note:** Phase 8 changes are on disk, not yet committed. Phase 7 commit `5a37213` and correction `342ec8d` are HEAD. Run `git diff` to see 8 modified + 7 new files (311+ inserts). - ---- - -## 2. Features Implemented - -### Partial Payments -- `ExpensesService.GetRemainingDebtAsync(payer, receiver, groupId): decimal` — net debt with `Math.Round(...,2,AwayFromZero)`. -- `ExpensesService.RegisterPayment(payer, receiver, groupId, amount)` — validates `00`, sum must equal `amount ±0.01` else return `[]` (prevent close), rounds each `amountOwed`. -- **Percentage:** `calculateSplyByPercentage` — filters, sumPct must be 100±0.01, each pct 0-100, then `amountOwed = round(pct/100*amount,2)`. -- Backend validation already `sum == amount ±0.02` and `AmountOwed>0`, now also covers percentage via same sum check. Fixed dialog template still uses `member.amount` (dynamic) correctly. - -### Monetary Precision -- Audited all `decimal(18,2)` operations: `ExpensesService.cs:249` `Math.Round(amount,2,AwayFromZero)`, `GroupService.cs` already `UtcNow`, `AddExpense` sum tolerance `0.02`, partial `0.01`. -- Tests with `10.01/3`, `33.33*3`, `100.01→33.33→66.68` etc. - -### Email Validation / Normalization -- Already `AuthService.cs:22` `Trim().ToLowerInvariant()`, `AnyAsync(u.Email.ToLower()==normalized)`, DTO `[EmailAddress][StringLength 100]`. No new verification flow (as requested, architecture prepared: `docs/SECURITY.md` notes token placeholder). Duplicate case-insensitive handled. - -### Group Admin -- Roles: `creator` (owner), `admin`, `member` (string). `GroupService.cs:105`: - - `IsUserAdminOrCreatorAsync`, `IsUserCreatorAsync` - - `UpdateMemberRoleAsync(groupId, target, newRole, requester)` — requester must be creator/admin, target not creator, not self, newRole admin|member, only creator can promote to admin. - - `RemoveMemberAsync` — creator can remove admin/member (not self/creator), admin can remove member only, member cannot. - - `DeleteGroupAsync` — only creator, cascades via FK. -- `GroupsController.cs:114` — `PUT /groups/{id}/members/{uid}/role` + `DELETE /members/{uid}` + `DELETE /groups/{id}` with `Forbid/BadRequest/NotFound` and `IsUserMember` check. -- Frontend `group-detail` `isAdminOrCreator` already checks `creator|admin`. - -### Application Admin -- Roles: `1 super`, `2 admin`, `3 user` (seed `Role` table). `UsersService.cs:23`: - - `GetAllUsersAsync()`, `IsUserAdminAsync(role 1/2)`, `UpdateUserRoleAsync(target, newRole, requester)` — super only, 1..3, not self. -- `AdminController.cs:1` — `[Authorize]` + manual `IsAdmin()` (`1|2`) for `GET /admin/users`, `IsSuperAdmin()` (`1`) for `PUT /admin/users/{id}/role`. Never trusts frontend `role`. -- Tests: `User→admin 403`, `Admin→admin 200`, `User→modify own role denied`, `Super→promote`. - ---- - -## 3. Business Rules Implemented - -``` -Partial Payments: - payment >0, payment <= remaining +0.01, payer!=receiver, both members, group exists - remaining = net payer->receiver - receiver->payer (rounded 2) - if remaining<=0 → throw "No debt" - if payment > remaining → throw "exceeds" - Distribution: oldest shares first, fully settle if share <= remainingPayment else reduce share.AmountOwed - Multiple payments accumulate, exact final → IsSettled, remaining 0 - Negative/zero → throw - -Split Methods: - Equal: per = floor(total/count*100)/100, remainder cents distributed to first N - Fixed: filtered amount>0, sum == total ±0.01 else invalid, each >0 - Percentage: filtered pct>0, sumPct ==100 ±0.01, each 0-100, amountOwed = round(pct/100*total,2), sum == total via backend - No negative allocations - -Monetary: - decimal(18,2), MidpointRounding.AwayFromZero, tolerance 0.01-0.02, remainder cents distribution - -Email: - trim, lowercase, EmailAddress, duplicate case-insensitive 409 Conflict - -Group Admin: - creator > admin > member - Only creator/admin can change roles; only creator can promote to admin; cannot change creator or self - Creator can remove admin/member; admin can remove member only; cannot remove creator - Only creator can delete group - -App Admin: - super(1) > admin(2) > user(3) - Only super can change roles; IsUserAdmin for GET /admin/users; never trust frontend role -``` - ---- - -## 4. Tests Added - -**Backend Unit (InMemory) — 33 new, total 78 passed +3 skipped =81 → now 78+? Let's recount: 81 previously, now 81 still? Actually new tests were already counted in 78. Now after Phase 8, total is 78+? Wait we added 33 earlier, now Phase 8 adds 33 more? Let's recount: `dotnet test` now 78 passed (same as before) — because new Phase 8 tests were already included in 78. Actually after Phase 8, `dotnet test` still 78, meaning new tests were already counted. Let's list:** - -- `PartialPaymentTests.cs:1` (7) — 30→70, multiple 30+20+50, exact 50, greater 30>20, zero/negative, no debt, multiple shares 60+40→70 -- `GroupAdminTests.cs:1` (9) — promote, admin cannot promote, member cannot, cannot promote creator, remove, admin remove, member cannot, delete, own role -- `AppAdminTests.cs:1` (6) — isAdmin, super promote, admin cannot, user cannot, own role, invalid -- `SplitMethodTests.cs:1` (6) — equal 100/3, fixed valid, fixed sum mismatch, percentage invalid 90/120/negative, percentage valid, negative -- `MonetaryPrecisionTests.cs:1` (5) — equal rounding 10.01/3 etc, tricky 100 with 3 participants, partial cents 100.01→33.33→66.68, boundary 0.01/0.02/1M - -**Frontend Unit — unchanged (5 specs, 25 SUCCESS). New split dialog logic covered via E2E, not yet new Karma specs (could add but E2E covers).** - -**E2E Playwright — 18 new in `e2e/phase8/` (all mocked, `serve -s`):** -- `partial-payments.spec.ts:1` (5) — 30→70, multiple, >debt 400, zero/negative 400, no debt 400 -- `split-methods.spec.ts:1` (4) — equal 100/3, fixed mismatch 400, percentage 90→400/100→201, negative 400 -- `group-admin.spec.ts:1` (5) — promote, member self, admin promote, remove, delete -- `app-admin.spec.ts:1` (4) — user 403, admin 200, user self 403, super promote - ---- - -## 5. Tests Executed - -```bash -dotnet test -c Release -→ 78 passed, 3 skipped (SkippableFact Docker not available), 0 failed, 81 total (39 previous + 42 new Phase 8) -# With Docker: 81 passed, 0 skipped - -npx ng test --watch=false --browsers ChromeHeadlessNoSandbox -→ 25 SUCCESS (karma.conf.js thresholds 45/20/30/45) - -npx playwright test --reporter=list (with serve) -→ 43 passed (25 Phase7 + 18 Phase8) — all mocked, no real API needed - # Phase7: 8 auth +3 groups +4 expenses +4 settlements +6 authz =25 - # Phase8: 5 partial +4 split +5 group-admin +4 app-admin =18 - -npm run build -→ success, dist/split-it-ui/browser, budget warn 592kB, sass @import deprecation -``` - ---- - -## 6. Coverage - -- **Backend:** `coverlet.runsettings` 70 line aspirational, `SplitIt.API` 80.5% line (prioritized). New partial/split/admin logic adds ~300 lines, coverage for `ExpensesService.cs` now includes `GetRemainingDebt` and partial loop, `GroupService.cs` admin, `UsersService` admin. Global `line-rate` still ~0.08 due to Domain, but security/business logic now ~85%. -- **Frontend:** `karma.conf.js` 45/20/30/45 global (downgraded from 70 to pass 51% statements). Phase 8 split dialog fix not yet covered by Karma (E2E covers), will rise with more specs in Phase 9. -- **E2E:** Not measured via coverlet, but mocked E2E covers all Phase 8 flows. - ---- - -## 7. Security / Authorization Tests - -| Test | Result | -|---|---| -| Partial payment > debt → 400 | `PartialPaymentTests.cs:48` `AppAdminTests` pass | -| Negative/zero payment → 400 | pass | -| No debt → 400 | pass | -| Fixed sum mismatch → 400 | `SplitMethodTests.cs:48` pass | -| Percentage sum≠100 → 400 | pass | -| Negative allocation → 400 | pass | -| Group Admin: member cannot promote → 403 | `GroupAdminTests.cs:48` + E2E `group-admin.spec.ts:27` 403 | -| Admin cannot promote to admin → 403 | `GroupAdminTests.cs:38` + E2E 403 | -| Remove creator → 400, admin remove member → 200, member cannot → 403 | pass | -| Only creator delete → 403/200 | `GroupAdminTests.cs:95` pass | -| App Admin: user → admin 403, super → promote 200 | `AppAdminTests.cs:14` + E2E `app-admin.spec.ts:8` | - ---- - -## 8. Known Limitations - -- **Partial Payments:** No `Payment` history UI yet (backend creates `Expense IsPayment` but frontend `settleDebt` still shows `settledCount`/`RemainingDebt` snackbar, not a payments list. Full payments list pending UI Phase 9. -- **Split Methods:** Frontend `split-method-dialog` now validates sum 100% and fixed sum, but does not show inline error messages (just prevents close). Better UX (error text) pending. -- **Email verification:** Not implemented (as requested, architecture prepared). No `verification token` yet. -- **Group Admin UI:** No buttons for promote/remove/delete in `group-detail.html` yet (backend ready, E2E mocked). UI will be added with Docker. -- **App Admin UI:** No Angular admin panel yet (backend `AdminController` ready). -- **Monetary:** Frontend still uses `double` (JS number) for `amount`, but rounds to 2dec; backend `decimal` correct. No `Money` value object yet. -- **Coverage:** Frontend 51% <70% aspirational, backend global 8% <70% (but business logic 85%). - ---- - -## 9. Remaining Risks - -- **MEDIUM:** Partial payment distribution across multiple shares ordered by date may not match business expectation if shares have different dates but same amount (currently oldest first, reasonable). -- **LOW:** `GroupMember` string roles not enum, but validated. -- **LOW:** No pagination in `GetAllUsers` for admin (could be large). - ---- - -## 10. Recommended Phase 9 Plan - -**Phase 9 — Docker (as requested, not yet):** - -```text -Backend Dockerfile (multi-stage, non-root, 8.0 runtime, healthcheck) -Frontend Dockerfile (node:22-alpine build → nginx:alpine serve or node serve) -SQL Server private network (no 1433 publish) -Nginx reverse proxy (not yet, Phase 10) -docker-compose.yml (api, sql, frontend, network splitit-net, volumes) -Health checks /health, /health/ready -.env.example already exists, use for compose -CI will build images, Trivy scan, not yet push -``` - -Do not start Phase 9 until this Phase 8 report is approved. - diff --git a/docs/PHASE_9_REPORT.md b/docs/PHASE_9_REPORT.md deleted file mode 100644 index 65406ad..0000000 --- a/docs/PHASE_9_REPORT.md +++ /dev/null @@ -1,73 +0,0 @@ -# Phase 9 — Docker Containerization Report - -## Status: COMPLETED - -All Phase 9 deliverables have been implemented and validated. - ---- - -## 1. Network Architecture - -Networks have been isolated and custom-named to avoid conflicts with other projects hosted on the same VPS: - -```text -INTERNET - │ - ▼ :80 / :443 -┌──────────────────┐ -│ splitit-frontend │ (Nginx + Angular SPA) -└────────┬─────────┘ - │ (splitit-frontend-net) - ▼ -┌──────────────────┐ -│ splitit-backend │ (.NET 8 Web API) -└────────┬─────────┘ - │ (splitit-backend-net: internal=true) - ▼ -┌──────────────────┐ -│ splitit-db │ (SQL Server 2022) -└──────────────────┘ -``` - -- **`splitit-frontend-net`**: Bridge network connecting frontend reverse proxy and API. -- **`splitit-backend-net`**: Fully internal bridge network (`internal: true`). SQL Server is accessible **only** to `splitit-backend`. Port `1433` is **not** exposed to the host machine. - ---- - -## 2. Security Implementations - -1. **Multi-Stage Build**: - - Backend: `dotnet/sdk:8.0` → `aspnet:8.0-alpine`. - - Frontend: `node:20-alpine` → `nginx:1.27-alpine`. -2. **Non-Root Container Execution**: - - Backend runs as `USER $APP_UID` (UID 1654). - - Frontend runs as `USER nginx`. -3. **Health Probes**: - - Backend: HTTP probe on `http://localhost:8080/health`. - - Frontend: HTTP probe on `http://localhost:80/`. - - SQL Server: `sqlcmd` query probe (`SELECT 1`). -4. **Resource Constraints**: - - SQL Server: 1.5 CPUs, 2 GB RAM. - - Backend API: 1.0 CPU, 512 MB RAM. - - Frontend Nginx: 0.5 CPU, 128 MB RAM. - ---- - -## 3. Files Created - -- `docker/backend/Dockerfile` -- `docker/frontend/Dockerfile` -- `docker/frontend/nginx.conf` -- `docker-compose.yml` -- `.dockerignore` -- `.env.example` -- `docs/DOCKER.md` -- `docs/PHASE_9_REPORT.md` - ---- - -## 4. Verification - -- `docker compose config`: Executed and validated successfully. -- `dotnet test`: 78 passed, 0 failed. -- Angular build: Succeeded without errors (`dist/split-it-ui/browser`). diff --git a/docs/PRODUCTION_AUDIT.md b/docs/PRODUCTION_AUDIT.md index ac30214..7e33ae0 100644 --- a/docs/PRODUCTION_AUDIT.md +++ b/docs/PRODUCTION_AUDIT.md @@ -350,9 +350,11 @@ Violación: Services en `Infrastructure` contienen lógica de negocio (debt calc --- -## 10. Exact Implementation Plan (fases obligatorias) +## 10. Historical Implementation Plan -> No se avanza de fase sin **reporte** `Changed files / Tests / Security improvements / Remaining risks / Next`. +> History only — kept to explain how the audit-driven work was sequenced. It is no longer a +> working process: each item is delivered as its own change and its durable outcome lives in +> `docs/specs/`, `docs/SECURITY.md`, the runbooks or an ADR. Per-phase reports are not maintained. ``` Phase 0 ✅ AUDIT (este documento) @@ -386,11 +388,11 @@ Phase 27 Final Quality Gate — checklist 27 ítems Phase 28 Docs — README, .env.example, ARCHITECTURE.md, etc. ``` -**Próximo paso inmediato (sin tu aprobación no se ejecuta):** +**Historical next step (from the audit):** **Phase 1 — Secrets & Configuration:** -- Crear `docs/SECURITY.md` parcial, `.env.example`, actualizar `.gitignore`, mover `JwtSettings` y `ConnectionStrings` a env vars con fallback, añadir `appsettings.Production.json` template, documentar rotación, instalar `gitleaks` pre-commit. - -¿Apruebas avanzar a Phase 1? Responde `sí` o indica ajustes a esta auditoría. +- Create `.env.example`, update `.gitignore`, move `JwtSettings` and `ConnectionStrings` to env vars + with fallback, add `appsettings.Production.json` template, document rotation in + `docs/SECURITY.md`. --- diff --git a/docs/REMEDIATION_REPORT_PHASE_0.5.md b/docs/REMEDIATION_REPORT_PHASE_0.5.md deleted file mode 100644 index 44826bc..0000000 --- a/docs/REMEDIATION_REPORT_PHASE_0.5.md +++ /dev/null @@ -1,178 +0,0 @@ -# SplitIt — Remediation Report Phase 0.5 (Critical Security) - -> **Fecha:** 2026-08-24 -> **Estado previo:** ⛔ NO APTO PARA PRODUCCIÓN (8 critical, 9 high) -> **Estado posterior:** 🟢 SEGURIDAD CRÍTICA REMEDIADA (0 critical abiertos) -> **Autor:** Muse Spark - -## 1. Changed Files (git status) - -``` -M SplitIt.API/SplitIt.API/appsettings.json -M SplitIt.API/SplitIt.API/Program.cs -M SplitIt.API/SplitIt.API/DependencyInjection.cs -M SplitIt.API/SplitIt.API/Controllers/AuthController.cs -M SplitIt.API/SplitIt.API/Controllers/GroupsController.cs -M SplitIt.API/SplitIt.API/Controllers/ExpensesController.cs -M SplitIt.API/SplitIt.Infrastructure/Services/AuthService.cs -M SplitIt.API/SplitIt.Infrastructure/Services/GroupService.cs -M SplitIt.API/SplitIt.Infrastructure/Services/ExpensesService.cs -M SplitIt.API/SplitIt.Application/DTOs/RegisterRequestDto.cs -M SplitIt.API/SplitIt.Application/DTOs/LoginRequestDto.cs -M SplitIt.API/SplitIt.Application/DTOs/CreateGroupDTO.cs -M SplitIt.API/SplitIt.Application/DTOs/ExpensesDTO.cs -M SplitIt.API/SplitIt.Application/DTOs/RegisterPaymentDto.cs -M SplitIt.API/SplitIt.Infrastructure/SplitIt.Infrastructure.csproj -M split-it-ui/src/app/modules/auth/guards/auth.guard.ts -M split-it-ui/src/app/interceptors/auth.interceptor.ts -M split-it-ui/src/app/modules/auth/services/auth.service.ts -M split-it-ui/src/app/modules/dashboard/dashboard.routes.ts -M .gitignore -M SplitIt.API/SplitIt.Back.sln -M docs/PRODUCTION_AUDIT.md - -A SplitIt.API/SplitIt.API/Middleware/GlobalExceptionHandler.cs -A SplitIt.API/SplitIt.API/appsettings.Development.json.example -A SplitIt.API/SplitIt.API/appsettings.Production.json.example -A SplitIt.Tests/SplitIt.Tests.csproj -A SplitIt.Tests/Helpers/TestDbHelper.cs -A SplitIt.Tests/AuthServicePasswordHashingTests.cs -A SplitIt.Tests/BolaTests.cs -A SplitIt.Tests/SettlementCrossGroupTests.cs -A SplitIt.Tests/JwtValidationTests2.cs -A SplitIt.Tests/ValidationTests.cs -A SplitIt.Tests/MassAssignmentTests.cs -A .env.example -A split-it-ui/.env.example -A split-it-ui/src/environments/environment.prod.ts.example -A docs/SECURITY.md -A docs/REMEDIATION_REPORT_PHASE_0.5.md (este archivo) -``` - -## 2. Security Fixes (detalle por hallazgo) - -### SEC-01 Secrets (C-01) -- **Before:** `appsettings.json:9-16` tenía `SuperSecretKey123...` + `Server=SANTIDEV21` committeado. -- **After:** Vaciado, templates `.example`, validación `Program.cs:23` falla en Prod si missing, `.gitignore` corr., doc rotación en `docs/SECURITY.md:1`. -- **Files:** `appsettings.json:1`, `appsettings.*.json.example:1`, `.env.example:1`, `Program.cs:17`, `DependencyInjection.cs:9`. - -### SEC-02 Password Hashing (C-02) -- **Before:** `SHA256(password)` en `AuthService.cs:49`. -- **After:** `IPasswordHasher` PBKDF2 + rehash legacy. Nuevos usuarios V3, login migra automáticamente. -- **Files:** `SplitIt.Infrastructure.csproj:10`, `AuthService.cs:1`. - -### SEC-03 AuthGuard (C-03) -- **Before:** `auth.guard.ts:3` `return true`. -- **After:** Verifica `localStorage token` + `exp` decode, `isTokenExpired`, redirect `/auth/login?returnUrl`, `dashboard.routes.ts:5` `canActivate`. -- **Files:** `auth.guard.ts:1`, `dashboard.routes.ts:1`, `auth.interceptor.ts:1` (401 cleanup), `auth.service.ts:46` logout fix. - -### SEC-04 BOLA/IDOR (C-04) -- **Before:** 6 endpoints sin `IsMember` check. -- **After:** `GroupService.IsUserMemberAsync` + `Forbid()` en todos `groupId` endpoints; `ExpensesService.AddExpenseAsync` valida membership + sum. -- **Files:** `GroupService.cs:100`, `GroupsController.cs:26,67,81,95`, `ExpensesController.cs:25,45,59,75`. - -### SEC-05 Settlement Cross-Group (H-05/C-04) -- **Before:** `SettleExpenseWithUser(payer,receiver)` sin `groupId`. -- **After:** Firma `SettleExpenseWithUser(payer,receiver,groupId)` scoped `GroupId==groupId`, `RegisterPayment` scoped, controller pasa `dto.GroupId`. -- **Files:** `ExpensesService.cs:202,235`, `ExpensesController.cs:86`. - -### SEC-06 CORS (C-05) -- **Before:** `AllowAnyOrigin`. -- **After:** `Cors:AllowedOrigins` CSV, `WithOrigins+AllowCredentials`, dev fallback `localhost:4200`, prod fail-closed. -- **Files:** `Program.cs:158`, `appsettings.json:14`, `.env.example:9`. - -### SEC-07 JWT (C-06) -- **Before:** `ASCII` vs `UTF8`, `ClockSkew 5m`, `RequireHttps false`. -- **After:** `UTF8`, `ClockSkew.Zero`, `RequireHttps=!IsDevelopment`, `ValidAlgorithms=[HS256]`, event alg check, `RequireSignedTokens+RequireExpirationTime`, secret fallback dev, `effectiveIssuer/Audience`. -- **Files:** `Program.cs:34,94`, `AuthController.cs:59`. - -### SEC-08 JWT Storage (C-07) -- **Decision:** Mantener `localStorage` + mitigaciones, documentado `docs/SECURITY.md:4`. No cookie aún para evitar CSRF mal implementado. -- **Files:** `docs/SECURITY.md:4`, `auth.interceptor.ts:1`. - -### SEC-09 Mass Assignment (C-08) -- **Before:** DTOs sin whitelist, riesgo `RoleId`. -- **After:** DTOs explícitos, `CreatedById` derivado JWT, `RoleId=3` hardcode server. -- **Files:** `RegisterRequestDto.cs:1`, `ExpensesDTO.cs:1`. - -### SEC-10 Input Validation (H-01) -- **Before:** Sin DataAnnotations. -- **After:** `[Required][StringLength][Range][EmailAddress]` en 5 DTOs + service checks `sum==Amount ±0.02`, `max 50`. -- **Files:** `CreateGroupDTO.cs:1`, `ExpensesDTO.cs:1`, etc. - -### SEC-11 Exception Handling (H-02) -- **Before:** `throw Exception`, sin middleware, stack leak. -- **After:** `GlobalExceptionHandler.cs:1` `IExceptionHandler`, `ProblemDetails` con `traceId`, no leak en prod. -- **Files:** `Middleware/GlobalExceptionHandler.cs:1`, `Program.cs:60,192`. - -### SEC-12 Rate Limiting (H-03) -- **Before:** Sin limit. -- **After:** `AddRateLimiter` `auth` 5/min/IP, `fixed` 100/min, `AuthController [EnableRateLimiting("auth")]`, `429` JSON. -- **Files:** `Program.cs:63`, `AuthController.cs:27,44`. - -## 3. Tests Added - -- **Framework:** xUnit, EF InMemory, Identity, `Microsoft.AspNetCore.Mvc.Testing` (preparado) -- **Location:** `SplitIt.Tests/` -- **Tests:** - - `AuthServicePasswordHashingTests` 5 — register hash prefix, correct/wrong password, legacy migrate, case-insensitive email. - - `BolaTests` 4 — not member, participant not member, sum mismatch. - - `SettlementCrossGroupTests` 2 — GroupA settle not affect GroupB, wrong group throw. - - `JwtValidationTests` 8 — valid, missing, tampered, expired, wrong iss/aud/sig, ClockSkew 0, none alg. - - `ValidationTests` 6 — register invalid/valid, expense amount, no participants, group name, payment zero. - - `MassAssignmentTests` 2 — extra RoleId/CreatedBy ignored. - -## 4. Tests Executed & Passed - -```text -dotnet test SplitIt.Tests -c Release -→ Passed! - Failed: 0, Passed: 33, Skipped: 0, Total: 33, Duration: 1 s -``` - -```text -dotnet build SplitIt.Back.sln -c Release -→ Compilación correcta. 0 Advertencias, 0 Errores - -npm run build (split-it-ui) -→ Output location: dist/split-it-ui — WARN budget 592.62kB >500kB, sass @import deprecation (non-blocking) -``` - -```text -dotnet list package --vulnerable -→ No vulnerable packages (NuGet) - -npm audit -→ 71 vulnerabilities (6 low, 21 mod, 40 high, 4 critical) via angular-devkit — documented, fix requires major bump Fase 14 -``` - -## 5. Remaining Vulnerabilities & Risks - -| Área | Riesgo | Severidad | Mitigación Fase 0.5 | Próxima Fase | -|---|---|---|---|---| -| XSS → localStorage theft | Si hay XSS almacenado, token robable | MEDIUM | Validation + interceptor 401, frontend sanitiza | CSP en Nginx Fase 11 + HttpOnly Fase 8 | -| No refresh revocation | JWT stolen válido 60m | MEDIUM | ClockSkew 0, 60m exp | Refresh rotation Fase 8 | -| npm audit 71 vulns | `webpack-dev-server` High | MEDIUM | Documentado, build ok | `npm audit fix` + Trivy Fase 14 | -| No pagination | DoS large groups | LOW | Limit 50 members/participants | Pagination Fase 25 | -| No Docker/CSP/HSTS/backup | Infra hardening pendiente | MEDIUM | No bloqueante security logic | Fases 9-13 | -| Swagger exposure | Si env prod mal config | LOW | `IsDevelopment()` gate | Protect/config Fase 24 | - -**Ningún 🔴 CRITICAL remanente. Cumple criterio Phase 0.5.** - -## 6. Verification Checklist (Phase 0.5 criteria) - -- [x] `appsettings.json` sin secretos -- [x] SHA256 eliminado, PBKDF2 + rehash -- [x] authGuard corr. + dashboard protected -- [x] BOLA checks en todos groupId endpoints -- [x] Settlement scoped groupId (test cross-group pass) -- [x] CORS sin AllowAnyOrigin en prod -- [x] JWT ClockSkew 0, UTF8, RequireHttps, alg check -- [x] JWT storage decisión doc -- [x] DTO validation -- [x] Global exception handler sin leak -- [x] Rate limiting auth 5/min (429) - -## 7. Next Phase - -**Esperando aprobación.** No continuar automáticamente. Próxima propuesta: **Phase 1 Secrets Hardening → Phase 9 Docker** o **Phase 6 Testing/e2e** según prioridad del usuario. Indicar `sí` para avanzar. - diff --git a/docs/adr/001-token-storage.md b/docs/adr/001-token-storage.md new file mode 100644 index 0000000..cd9c21b --- /dev/null +++ b/docs/adr/001-token-storage.md @@ -0,0 +1,28 @@ +# ADR-001 — Store the JWT in `localStorage` + +- **Status:** Accepted +- **Date:** 2026-08-24 + +## Context + +The Angular SPA must attach a JWT to API calls and keep the session across reloads. The token needs +a client-side home that survives navigation. + +## Options + +1. `localStorage` — simplest; readable by any script on the page, so a successful XSS can steal it. +2. `HttpOnly` cookie — not readable by JS (XSS cannot exfiltrate it) but introduces CSRF and forces + `SameSite` + anti-CSRF handling. + +## Decision + +Keep the token in `localStorage`. Treat it as an accepted, documented risk and make XSS prevention +non-negotiable: strict input validation, a restrictive CSP at the proxy, and no `innerHTML` on +untrusted data. + +## Consequences + +- A stored XSS could exfiltrate the token; the compensating control is strict XSS prevention plus + CSP, not the storage mechanism. +- Moving to `HttpOnly` cookies later must add CSRF protection in the same change — never do half of + it. diff --git a/docs/adr/002-framework-baseline.md b/docs/adr/002-framework-baseline.md new file mode 100644 index 0000000..eeb7f7c --- /dev/null +++ b/docs/adr/002-framework-baseline.md @@ -0,0 +1,24 @@ +# ADR-002 — Angular 21 and .NET 8 baseline + +- **Status:** Accepted +- **Date:** 2026-09-21 + +## Context + +The project needs a frontend and backend stack that a small team can move fast with and still +deploy safely to a single VPS. + +## Options + +1. Track the newest framework releases aggressively. +2. Stay one or two releases behind on a supported LTS baseline and upgrade in dedicated changes. + +## Decision + +Use **Angular 21** (Material, SCSS) and **.NET 8** with Clean Architecture. Treat framework +upgrades as their own dedicated change, never mixed into a feature. + +## Consequences + +- Angular 19 is out of support; an upgrade past the current release is planned work, not a surprise. +- Features stay reviewable because a diff is either a feature or an upgrade, never both. diff --git a/docs/adr/003-monetary-rounding.md b/docs/adr/003-monetary-rounding.md new file mode 100644 index 0000000..53d6171 --- /dev/null +++ b/docs/adr/003-monetary-rounding.md @@ -0,0 +1,26 @@ +# ADR-003 — Monetary rounding and split distribution + +- **Status:** Accepted +- **Date:** 2026-08-24 + +## Context + +Splitting an amount across participants rarely divides evenly (`100/3`), and floating point makes +"the parts must add up to the whole" a real requirement for a money feature. + +## Options + +1. Let each client compute shares — simple, but two clients can disagree and totals drift. +2. Compute and validate on the server with integer cents and a defined rounding rule; the client + mirrors the logic but never owns it. + +## Decision + +Amounts are `decimal(18,2)`; rounding is `MidpointRounding.AwayFromZero`; sum tolerances are +`0.01`–`0.02`. The **backend is the source of truth** for every split; the frontend mirrors the +logic, and the backend re-validates the sum on submit. + +## Consequences + +- Every split sums to exactly the total; the odd cent goes to the first members by a fixed rule. +- The frontend can never be trusted to be the only validator. diff --git a/docs/adr/004-deployment-topology.md b/docs/adr/004-deployment-topology.md new file mode 100644 index 0000000..d4c3b8e --- /dev/null +++ b/docs/adr/004-deployment-topology.md @@ -0,0 +1,29 @@ +# ADR-004 — Deployment topology (single VPS behind `vps-gateway`) + +- **Status:** Accepted +- **Date:** 2026-09-14 + +## Context + +SplitIt runs on one VPS shared with other projects. TLS, routing and the public surface must be +consistent across projects, and the database must stay unreachable from the internet. + +## Options + +1. Expose each app directly with its own TLS and port — repeated TLS config, more open ports, + inconsistent headers. +2. One reverse-proxy gateway terminates TLS and routes by `server_name`; each app publishes no host + ports and attaches to a shared network. + +## Decision + +Deploy behind the **`vps-gateway`** reverse proxy (path `docs/specs/architecture.md`). The gateway +owns :80/:443 and TLS; SplitIt attaches to `splitit-net` (gateway ingress) and keeps SQL Server on +`splitit-internal-net` (`internal: true`), never publishing host ports. + +## Consequences + +- No per-app TLS or public ports; headers/rate limits are configured per site at the gateway. +- Every new project follows the same `vps-gateway` procedure (DNS → cert → site → reload). +- Testing against the real routing requires the gateway, so local dev uses the app's own proxy + instead. diff --git a/docs/adr/README.md b/docs/adr/README.md new file mode 100644 index 0000000..ecdf290 --- /dev/null +++ b/docs/adr/README.md @@ -0,0 +1,9 @@ +# Architecture Decision Records + +One file per irreversible or costly decision: `NNN-title.md` with **Context**, **Options**, +**Decision** and **Consequences**. Written by the human, in a few paragraphs — not generated as a +report. + +Use an ADR when a choice is hard to reverse or future-you needs the *why* (storage, security +trade-offs, schema strategy, protocol). For everyday changes, update the relevant +[spec](../specs/) and the code instead. diff --git a/docs/specs/business-rules.md b/docs/specs/business-rules.md new file mode 100644 index 0000000..6744f90 --- /dev/null +++ b/docs/specs/business-rules.md @@ -0,0 +1,54 @@ +# Business Rules + +Current behavior, not a report. Derived from the Phase 8 feature work; this is the durable spec. + +## Partial payments + +- A payment is an `Expense` with `IsPayment = true` plus a settled `ExpenseShare`. +- `GetRemainingDebtAsync(payer, receiver, groupId)` = net debt `payer→receiver` minus + `receiver→payer`, rounded to 2 decimals (`MidpointRounding.AwayFromZero`). +- `RegisterPayment(payer, receiver, groupId, amount)` validates: `0 < amount <= remaining + 0.01`, + `payer != receiver`, both are group members, group exists. Zero/negative and "no debt" are + rejected. +- Distribution: the payment settles the payer's unsettled shares oldest-first (`Expense.Date`); a + share is fully settled when `AmountOwed <= remainingPayment`, otherwise reduced by the remainder. + Payments accumulate to an exact zero remaining. +- `POST /api/expenses/settle` handles direction swapping (tries `payer→receiver`, then reversed, + uses `Math.Abs`) and returns `{ PaymentId, RemainingDebt, SettledCount }`. + `GET /api/expenses/remaining-debt?otherUserId&groupId` feeds the UI. + +## Split methods + +- **Equal:** `perPerson = floor(total / count * 100) / 100`; the remainder (in cents) is distributed + `+0.01` to the first members so the sum is exactly `total` (e.g. `100/3 → 33.34, 33.33, 33.33`). +- **Fixed:** only members with `amount > 0`; the sum must equal `total ± 0.01` or the split is + rejected. +- **Percentage:** only members with `pct > 0`; the percentages must sum to `100 ± 0.01`, each + `0–100`; `amountOwed = round(pct/100 * total, 2)`; the backend re-checks the sum. +- No negative or zero allocations. + +## Monetary precision + +- Amounts are `decimal(18,2)`; rounding is `MidpointRounding.AwayFromZero`; sum tolerances are + `0.01`–`0.02`. The frontend uses JS numbers but rounds to 2 decimals — the backend is the source + of truth. + +## Group roles + +- `creator > admin > member`. +- Only creator/admin change roles; only the creator promotes to admin; the creator's role and one's + own role cannot be changed. +- Creator removes admin/member; admin removes member only; nobody removes the creator. Only the + creator deletes the group. + +## Application roles + +- `1 super > 2 admin > 3 user` (seeded `Role` table). +- Only a super admin changes roles; `GET /api/admin/users` requires admin; + `PUT /api/admin/users/{id}/role` requires super. The server never trusts a client-supplied role; + `RoleId` is assigned server-side on register. + +## Email + +- Trimmed and lowercased; `[EmailAddress]`/length validated; duplicates are rejected + case-insensitively (409).