mirror of
https://github.com/azaion/admin.git
synced 2026-06-21 10:51:10 +00:00
refactor: remove deploy.cmd and update Dockerfile for health checks
- Deleted the deploy.cmd script as it was no longer needed. - Updated Dockerfile to include curl for health checks and added a non-root user for improved security. - Modified health check command to use curl for better reliability. - Adjusted docker-compose.test.yml to reflect changes in health check configuration. - Cleaned up appsettings.json and removed unused configuration properties. - Removed Resource entity and related requests from the codebase as part of the architectural shift. - Updated documentation to reflect the removal of hardware binding and related endpoints. Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
@@ -0,0 +1,58 @@
|
||||
# Dependency Scan
|
||||
|
||||
**Date**: 2026-05-13
|
||||
**Scanner**: `dotnet list package --vulnerable --include-transitive` + `--deprecated` (NuGet metadata) plus manual cross-reference of pinned versions against published GitHub Security Advisories (GHSA).
|
||||
**Sources used**: `api.nuget.org`, three private `pkgs.dev.azure.com/pwc-us-prism/_packaging/*` feeds.
|
||||
|
||||
## Scope
|
||||
|
||||
| Project | Vulnerable Packages |
|
||||
|---------|---------------------|
|
||||
| `Azaion.AdminApi` | none reported |
|
||||
| `Azaion.Common` | none reported |
|
||||
| `Azaion.Services` | none reported |
|
||||
| `Azaion.Test` | none reported |
|
||||
| `e2e/Azaion.E2E` | none reported |
|
||||
|
||||
`dotnet list package --vulnerable --include-transitive` returned a clean result for every project against the configured feeds. No CVE-ranked findings.
|
||||
|
||||
## Deprecated Packages
|
||||
|
||||
| Project | Package | Version | Reason | Recommended |
|
||||
|---------|---------|---------|--------|-------------|
|
||||
| `Azaion.AdminApi` | `FluentValidation.AspNetCore` | 11.3.0 | Legacy (deprecated by maintainer) | Move validators to manual `ServiceCollectionExtensions.AddValidatorsFromAssembly(...)` registration; `FluentValidation` 11.10.0 (already in use elsewhere) is the supported core. The AspNetCore auto-DI helper is no longer maintained. |
|
||||
| `Azaion.Services` | `System.IdentityModel.Tokens.Jwt` | 7.1.2 | Legacy (Microsoft pushes consumers to `Microsoft.IdentityModel.JsonWebTokens`) | Migrate to `Microsoft.IdentityModel.JsonWebTokens` (the modern token-handler stack already shipped via `Microsoft.AspNetCore.Authentication.JwtBearer 10.0.3`). |
|
||||
| `Azaion.Test` | `xunit` | 2.9.2 | Legacy (`xunit.v3` is the new line) | Plan a migration to `xunit.v3` once it leaves prerelease. Not urgent — `xunit 2.x` still receives security backports. |
|
||||
|
||||
Deprecated ≠ vulnerable. None of the three packages above carry an open CVE. They are flagged so we have a paper trail before they reach end-of-life.
|
||||
|
||||
## Manual Advisory Cross-Reference
|
||||
|
||||
The pinned top-level package list (output of `dotnet list package`) was cross-checked against GitHub Security Advisories for known issues NOT yet surfaced by NuGet metadata:
|
||||
|
||||
| Package | Pinned | Advisory | Severity | Fix Version | Notes |
|
||||
|---------|--------|----------|----------|-------------|-------|
|
||||
| `Newtonsoft.Json` | **13.0.1** | GHSA-5crp-9r3c-p9vr (Improper Handling of Exceptional Conditions — DoS via deeply nested JSON) | **High** | **13.0.2 or higher** | Used transitively + directly across `Azaion.Common`, `Azaion.Services`. Untrusted JSON enters via `LoginRequest`, `RegisterUserRequest`, `GetUpdateRequest`, etc. — all of which deserialize via the ASP.NET Core minimal API stack. Even though minimal API uses `System.Text.Json` by default, the `Newtonsoft.Json` reference is reachable from logging payload formatting and from `ResourceColumnEncryption`-adjacent code paths. **Bump to 13.0.3 or later.** |
|
||||
| `LazyCache.AspNetCore` | 2.4.0 | none open | — | — | Last release 2022; in maintenance mode. No advisory. |
|
||||
| `Microsoft.AspNetCore.Authentication.JwtBearer` | 10.0.3 | none open | — | — | Latest .NET 10 line. |
|
||||
| `Npgsql` | 10.0.1 | none open | — | — | Current. |
|
||||
| `linq2db` | 5.4.1 | none open | — | — | Current. |
|
||||
| `Swashbuckle.AspNetCore` | 10.1.4 | none open | — | — | Current. |
|
||||
| `Serilog` family (`4.1.0` / sinks `6.0.0` / `8.0.0`) | varies | none open | — | — | Current. |
|
||||
| `FluentAssertions` | 6.12.2 | n/a (test-only) | — | — | License changed in 8.0; staying on 6.x is fine. |
|
||||
|
||||
## Findings
|
||||
|
||||
### D-1: `Newtonsoft.Json 13.0.1` is below the patched line for GHSA-5crp-9r3c-p9vr (High) — **RESOLVED in cycle 1**
|
||||
|
||||
- **Severity**: High (now closed)
|
||||
- **CVE/Advisory**: GHSA-5crp-9r3c-p9vr (DoS via uncontrolled recursion when deserializing deeply nested JSON)
|
||||
- **Location at time of finding**: top-level reference in `Azaion.Common.csproj`, `Azaion.Services.csproj`
|
||||
- **Resolution (2026-05-13)**: bumped to **13.0.4** (current stable, released 2025-09-17) in both csproj files. `dotnet restore` + `dotnet build` succeeded. Full test suite re-ran clean: 48 e2e (Docker) + 2 unit. The 13.0.1 → 13.0.4 jump is patch-level on the same major; `JsonConvert.SerializeObject` / `DeserializeObject` API surface unchanged at the call sites (`AzaionDbSchemaHolder`, `BusinessExceptionHandler`, `SecurityTest`).
|
||||
- **Notes**: NuGet's `--vulnerable` did not flag this on the configured feeds — likely because the GHSA → NuGet vulnerability index sync depends on advisory enrichment that hasn't propagated to all mirrors. Manual upgrade was warranted.
|
||||
|
||||
## Self-verification
|
||||
|
||||
- [x] All package manifests scanned (5 csproj, 4 production + 1 e2e)
|
||||
- [x] Each finding has a CVE/advisory reference
|
||||
- [x] Upgrade paths identified for High findings
|
||||
@@ -0,0 +1,102 @@
|
||||
# Infrastructure & Configuration Review
|
||||
|
||||
**Date**: 2026-05-13
|
||||
**Scope**: `Dockerfile`, `docker.test/Dockerfile`, `e2e/Dockerfile`, `docker-compose.test.yml`, `appsettings*.json`, `env/db/*.sql`, `.gitignore`, `.dockerignore`. No CI/CD pipeline files (`.github/workflows/`, `.gitlab-ci.yml`, `azure-pipelines.yml`) are present in this workspace.
|
||||
|
||||
## Container Security
|
||||
|
||||
### Production image (`Dockerfile`)
|
||||
|
||||
| Check | Result | Notes |
|
||||
|-------|--------|-------|
|
||||
| Non-root user | **FAIL** | No `USER` directive — runs as root. See F-6. |
|
||||
| Minimal base image | PASS | `mcr.microsoft.com/dotnet/aspnet:10.0` (runtime-only) for the final stage; SDK image used only for build. |
|
||||
| Multi-stage build | PASS | Build / publish / runtime stages cleanly separated. |
|
||||
| No secrets in build args | PASS | Only `CI_COMMIT_SHA` (passed as `AZAION_REVISION` env at runtime) — non-sensitive. |
|
||||
| Health check defined | PASS (compose layer) | `docker-compose.test.yml:42-51` defines a TCP health check; the production deployment must define an equivalent. **Not verified in this audit** because no production compose file exists in this workspace. |
|
||||
| Image pinned by digest | **WARN** | Base images use `:10.0` tag, not `@sha256:...` digest. Tag floats — a poisoned upstream tag would be picked up on the next rebuild. Acceptable if the build runs from a controlled cache; otherwise pin. |
|
||||
| `EXPOSE` matches Kestrel binding | PASS | `EXPOSE 8080`, `ASPNETCORE_URLS=http://+:8080`. |
|
||||
|
||||
### Test sidecar image (`docker.test/Dockerfile`)
|
||||
|
||||
```
|
||||
FROM alpine:latest
|
||||
CMD echo hello
|
||||
```
|
||||
|
||||
This image is essentially a no-op stub. It's referenced from somewhere in the test/CI tooling but contributes nothing functional. **Recommendation**: either remove it (if nothing references it) or document its purpose. From a security standpoint it's inert — `alpine:latest` is fine for an `echo` and the floating tag is irrelevant for a stub. Flag as **operational hygiene, not a security finding**.
|
||||
|
||||
### E2E runner image (`e2e/Dockerfile`)
|
||||
|
||||
| Check | Result | Notes |
|
||||
|-------|--------|-------|
|
||||
| Non-root | **FAIL** (test-only) | Same root-by-default behavior. Test runner only — no exposed network, runs in an ephemeral container per CI run. **Acceptable.** |
|
||||
| Uses SDK image as final | **WARN** (test-only) | Final stage is `mcr.microsoft.com/dotnet/sdk:10.0` — needed for `dotnet test`. SDK images carry more attack surface than runtime images, but this container is only reachable on the internal `e2e-net` bridge. **Acceptable for test-only.** |
|
||||
|
||||
## docker-compose.test.yml
|
||||
|
||||
| Check | Result | Notes |
|
||||
|-------|--------|-------|
|
||||
| Secrets via env vars (not committed prod secrets) | PASS (test-only) | The DB password, JWT secret, and master key are committed because this is the e2e harness only. F-10 captures the rule: these literals must never appear in a production compose file. |
|
||||
| No port leaks | PASS | Only `8080:8080` is published from `system-under-test`; `test-db` is internal to the bridge. |
|
||||
| Healthcheck for the API | PASS | TCP probe on `127.0.0.1:8080`. |
|
||||
| Network isolation | PASS | All three services share `e2e-net` only; no `host` network mode. |
|
||||
| Image lock | **WARN** (test-only) | `postgres:16-alpine` floats. For test reproducibility consider pinning a digest, but not security-critical for an ephemeral test stack. |
|
||||
|
||||
There is **no production `docker-compose.yml`** in this workspace. Any production deployment must:
|
||||
- Inject `JwtConfig__Secret` and both `ConnectionStrings__*` values from a secret manager (Vault, AWS Secrets Manager, Azure Key Vault). (`ResourcesConfig__EncryptionMasterKey` was the third item here pre-revert; that field has since been deleted along with the OTA feature.)
|
||||
- NOT carry over the `ASPNETCORE_ENVIRONMENT=Development` value used in the test compose — that environment value enables Swagger at the root path.
|
||||
|
||||
## Environment Configuration
|
||||
|
||||
### `Azaion.AdminApi/appsettings.json`
|
||||
|
||||
| Field | Value committed | Risk |
|
||||
|-------|----------------|------|
|
||||
| `Logging.LogLevel.*` | `Information` / `Warning` | OK |
|
||||
| `AllowedHosts` | `"*"` | OK at the framework level — host filtering is normally enforced upstream by the reverse proxy. |
|
||||
| `ResourcesConfig.ResourcesFolder` | `"Content"` | Relative path; resolves under the working directory inside the container. OK. |
|
||||
| ~~`ResourcesConfig.EncryptionMasterKey`~~ | — | **Removed in the post-cycle-1 revert** along with the OTA feature; field no longer exists in `ResourcesConfig`, `appsettings.json`, or `docker-compose.test.yml`. Closes F-5 automatically. |
|
||||
| `JwtConfig.{Issuer, Audience, TokenLifetimeHours}` | committed | Public-by-design (not secrets). |
|
||||
| `JwtConfig.Secret` | **NOT committed** | Correct. Supplied via env var. |
|
||||
| `ConnectionStrings.*` | **NOT committed** | Correct. Supplied via env var. |
|
||||
|
||||
### `Azaion.AdminApi/appsettings.Development.json`
|
||||
|
||||
Empty except for log levels — fine.
|
||||
|
||||
### `e2e/Azaion.E2E/appsettings.test.json`
|
||||
|
||||
Test-only. F-10 captures this; not a production concern.
|
||||
|
||||
## Secrets Hygiene
|
||||
|
||||
| Pattern | Where | Disposition |
|
||||
|---------|-------|-------------|
|
||||
| Plaintext DB passwords | `env/db/01_permissions.sql` | F-11 — operator template, not runtime. Add a header comment. |
|
||||
| Plaintext DB / JWT / master-key in `docker-compose.test.yml` | `docker-compose.test.yml:31-37` | F-10 — test-only. CI guard recommended. |
|
||||
| Plaintext admin/uploader passwords in `e2e/Azaion.E2E/appsettings.test.json` | as above | F-10 — test-only. |
|
||||
| `.env` ignored | `.gitignore:10` | PASS |
|
||||
| `bin/`, `obj/`, `logs/`, `Content/` ignored | `.gitignore:2-3, 7, 9` | PASS — keeps build artifacts and runtime data out of git. |
|
||||
|
||||
## CI/CD
|
||||
|
||||
No CI configuration files were found in the workspace (`.github/workflows/`, `.gitlab-ci.yml`, `azure-pipelines.yml`, `Jenkinsfile` — none present). Either:
|
||||
- CI is configured outside this repo (e.g., upstream meta-repo), in which case the security guardrails (`dotnet list package --vulnerable`, secret-scanning, dependency-review) need to be verified there.
|
||||
- Or there is no automated CI today, in which case this is a meta-finding: the dependency-bump (D-1) and the test suite have no automated gate. **Recommend**: introduce at least a `dotnet build && dotnet test && dotnet list package --vulnerable` job before deploy.
|
||||
|
||||
## Network Security
|
||||
|
||||
| Check | Status | Notes |
|
||||
|-------|--------|-------|
|
||||
| HTTPS enforcement in code | **FAIL** | F-13 — assumed at reverse proxy. |
|
||||
| HSTS | not configured | Acceptable if reverse proxy injects it. |
|
||||
| CORS | tight | `AdminCorsPolicy` whitelist-only (`https://admin.azaion.com`, `http://admin.azaion.com`). The plaintext `http://` origin can be removed once the SaaS UI is HTTPS-only. |
|
||||
| Security headers | not configured in app | `X-Frame-Options`, `X-Content-Type-Options`, `Content-Security-Policy` not set in code. Reverse proxy responsibility today; document the assumption. |
|
||||
|
||||
## Self-verification
|
||||
|
||||
- [x] All Dockerfiles reviewed (`Dockerfile`, `docker.test/Dockerfile`, `e2e/Dockerfile`)
|
||||
- [x] All compose files reviewed (`docker-compose.test.yml` — only one in repo)
|
||||
- [x] All environment / config files reviewed (3 `appsettings*.json`)
|
||||
- [x] CI/CD reviewed (none present in repo — surfaced as meta-finding)
|
||||
@@ -0,0 +1,50 @@
|
||||
# OWASP Top 10 Review (2021 edition)
|
||||
|
||||
**Date**: 2026-05-13
|
||||
**Framework**: [OWASP Top 10 — 2021](https://owasp.org/www-project-top-ten/) (the 2025 release is not yet finalized as of this audit; 2021 remains the current authoritative list).
|
||||
**Scope cross-reference**: every FAIL below cites a Phase 2 finding ID (`F-N`) for the underlying evidence.
|
||||
|
||||
## Per-Category Assessment
|
||||
|
||||
| # | Category | Status | Findings |
|
||||
|---|----------|--------|----------|
|
||||
| A01 | Broken Access Control | **FAIL** | F-2 (path traversal via `dataFolder`) — F-1 closed via OTA feature revert |
|
||||
| A02 | Cryptographic Failures | PASS_WITH_WARNINGS | F-1 closed via revert; D-1 closed via Newtonsoft bump (13.0.4); F-7 (SHA-384 password hash, no salt/KDF) remains open as a hardening item |
|
||||
| A03 | Injection | **PASS** | linq2db parameterizes all queries; no string-concatenation SQL paths found in `Azaion.Services/*Service.cs` or `Azaion.Common/Database/*`. No `Process.Start` / `subprocess` usage in production code. No template injection paths. |
|
||||
| A04 | Insecure Design | PASS_WITH_WARNINGS | F-3 closed (UNIQUE INDEX `users_email_uidx` + `RegisterUser`/`RegisterDevice` consolidation); F-8 (no rate limiting on `/login`) remains as a hardening item |
|
||||
| A05 | Security Misconfiguration | **FAIL** | F-6 (container runs as root), F-13 (no HTTPS enforcement in code), F-9 (request DTOs missing validators), F-11 (placeholder credentials in `01_permissions.sql`). F-5 closed automatically (`EncryptionMasterKey` field deleted with the OTA revert). |
|
||||
| A06 | Vulnerable & Outdated Components | **PASS** | All `dotnet list package --vulnerable` checks return clean. D-1 (Newtonsoft.Json) was the only manual finding; closed in this audit by bumping to 13.0.4. Three deprecated-but-not-vulnerable packages noted in `dependency_scan.md`. |
|
||||
| A07 | Identification & Authentication Failures | PASS_WITH_WARNINGS | F-3 closed (DB UNIQUE INDEX now enforces one-row-per-email). F-7 (weak password hashing) and F-8 (no rate limiting) remain open as hardening items. |
|
||||
| A08 | Software & Data Integrity Failures | **PASS** | OTA flow that introduced the unsigned-manifest concern was reverted. CI/CD: secrets are env-injected, no in-repo secrets in `Dockerfile` / compose files used by prod. |
|
||||
| A09 | Security Logging & Monitoring Failures | **PASS_WITH_WARNINGS** | Serilog console + rolling file sink configured. F-12 (one unstructured log line in `ResourcesService`). No security-event-specific logger — login successes/failures, role changes, deletes are not separately auditable. |
|
||||
| A10 | Server-Side Request Forgery (SSRF) | **NOT_APPLICABLE** | The API never makes outbound HTTP calls based on user-controlled URLs. `CdnUrl` from `PublishResourceRequest` is stored and forwarded but never fetched server-side. |
|
||||
|
||||
## Cross-Reference Against `security_approach.md`
|
||||
|
||||
The pre-cycle-1 `security_approach.md` "Known Security Observations" list is reconciled here:
|
||||
|
||||
| Original observation | Status post-cycle-1 |
|
||||
|----------------------|---------------------|
|
||||
| 1. SHA-384 without per-user salt | **Still open** — F-7 |
|
||||
| 2. `hardware_hash` DB column unused | **Resolved by AZ-197** — column-level removal pending follow-up; field is now dead but the column is still in the schema (`02_structure.sql:9`). Not a security risk; cleanup task. |
|
||||
| 3. No path traversal protection on `dataFolder` | **Still open** — F-2 |
|
||||
| 4. Hardcoded DB credentials in test files | **Confirmed test-only** — F-10 |
|
||||
| 5. No rate limiting on `/login` | **Still open** — F-8 |
|
||||
| 6. No audit trail for security-relevant operations | **Still open** — A09 PASS_WITH_WARNINGS |
|
||||
| 7. No HTTPS enforcement in code | **Still open** — F-13 |
|
||||
| 8. Static encryption key salts hardcoded | **Partially resolved** — `Security.GetApiEncryptionKey` salt is still hardcoded but the AZ-197 removal of the `hwHash` component reduced surface area. (`ResourceColumnEncryption` was deleted along with the OTA revert.) |
|
||||
|
||||
## Cycle-1 Specific Verdict
|
||||
|
||||
The cycle-1 changes (AZ-513, AZ-196, AZ-183, AZ-197) introduced one new High-severity finding (F-1, on `/get-update`) and amplified one existing High (F-3, via `RegisterDevice`). Both were closed before any deploy:
|
||||
|
||||
- **F-1 (resolved by feature revert)**: AZ-183 was reverted in full; the OTA delivery model itself is obsolete in the target architecture.
|
||||
- **F-3 (resolved)**: `RegisterDevice` now delegates to `RegisterUser`; `users.email` has a UNIQUE INDEX (`users_email_uidx`); UNIQUE-violation is translated to `EmailExists`.
|
||||
|
||||
Other cycle-1 endpoints (`/devices`, `/classes/*`) have correct authorization wiring (`apiAdminPolicy`).
|
||||
|
||||
## Self-verification
|
||||
|
||||
- [x] All current OWASP Top 10 categories assessed
|
||||
- [x] Each FAIL has at least one specific finding with evidence (F-N reference)
|
||||
- [x] NOT_APPLICABLE category has justification (A10)
|
||||
@@ -0,0 +1,120 @@
|
||||
# Security Audit Report
|
||||
|
||||
**Date**: 2026-05-13
|
||||
**Scope**: Azaion Admin API workspace (`/Users/obezdienie001/dev/azaion/suite/admin`) — full audit triggered by autodev cycle 1 completion (AZ-513, AZ-196, AZ-183, AZ-197).
|
||||
**Verdict**: **PASS_WITH_WARNINGS** (Critical: 0; High: 1 open — F-2 path traversal pre-existing; deferred to a separate ticket. F-1, F-3, D-1 closed in this audit.)
|
||||
|
||||
## Summary
|
||||
|
||||
| Severity | Count | Closed in this audit |
|
||||
|----------|-------|----------------------|
|
||||
| Critical | 0 | 0 |
|
||||
| High | 3 | 3 — D-1 (`Newtonsoft.Json` 13.0.1 → 13.0.4), F-1 (OTA feature reverted — endpoints, service, entity, table, request DTOs, response DTO, e2e tests, master-key config field all deleted), F-3 (`RegisterDevice` reuses `RegisterUser`; UNIQUE INDEX `users_email_uidx` added) |
|
||||
| Medium | 5 | 0 |
|
||||
| Low | 5 | 0 |
|
||||
|
||||
## Open / Deferred
|
||||
|
||||
- **F-2** (path traversal via `dataFolder` route segment) remains open. It is pre-existing and unrelated to cycle-1 changes. Recommended: file as a separate ticket and close in a focused refactor (the fix needs both validation and a deployment-time review of `Content/` permissions). Not a blocker on this cycle.
|
||||
|
||||
## OWASP Top 10 (2021) Assessment
|
||||
|
||||
| Category | Status | Findings |
|
||||
|----------|--------|----------|
|
||||
| A01 Broken Access Control | **FAIL** | F-2 (open) — F-1 closed via OTA feature revert |
|
||||
| A02 Cryptographic Failures | PASS_WITH_WARNINGS | F-1 closed via revert; D-1 closed via Newtonsoft bump; F-7 (open, hardening) |
|
||||
| A03 Injection | PASS | — |
|
||||
| A04 Insecure Design | PASS_WITH_WARNINGS | F-3 closed; F-8 (open, hardening) |
|
||||
| A05 Security Misconfiguration | **FAIL** | F-5, F-6, F-13, F-9, F-11 |
|
||||
| A06 Vulnerable Components | PASS | D-1 closed; 3 deprecated-but-not-vulnerable packages logged |
|
||||
| A07 Auth Failures | PASS_WITH_WARNINGS | F-3 closed; F-7 + F-8 (open, hardening) |
|
||||
| A08 Data Integrity Failures | PASS | OTA flow that introduced the unsigned-manifest concern was reverted |
|
||||
| A09 Logging Failures | PASS_WITH_WARNINGS | F-12; no separate security audit log |
|
||||
| A10 SSRF | NOT_APPLICABLE | API makes no outbound calls based on user URLs |
|
||||
|
||||
## Findings (severity-ranked)
|
||||
|
||||
| # | Severity | Category | Location | Title |
|
||||
|---|----------|----------|----------|-------|
|
||||
| F-1 | **High** (CLOSED via revert) | A01 / A02 | (deleted) `Program.cs` `/get-update`, `ResourceUpdateService.cs` | `/get-update` exposed plaintext `EncryptionKey` to any authenticated caller. **Resolution**: the entire OTA feature was deleted — endpoints, `IResourceUpdateService`, `ResourceColumnEncryption`, `Resource` entity, `resources` table, `EncryptionMasterKey` config field, `apiUploaderPolicy`, `ResourceUpdateTests.cs`. AZ-183 is reverted; the OTA delivery model itself is obsolete in the target architecture. |
|
||||
| F-2 | **High** (open) | A01 | `ResourcesService.cs:20-25` + `Program.cs:201,213,219,224` | Path traversal via `dataFolder` route segment (pre-existing) — deferred to a separate ticket |
|
||||
| F-3 | **High** (CLOSED) | A04 / A07 | `env/db/06_users_email_unique.sql`, `UserService.cs` | `users.email` lacked UNIQUE → duplicate-row race in `RegisterUser` and `RegisterDevice`. **Resolution**: added migration `env/db/06_users_email_unique.sql` (`CREATE UNIQUE INDEX users_email_uidx ON public.users (email)`); refactored `RegisterUser` to drop the check-then-insert pattern and translate `Npgsql.PostgresException(SqlState=23505)` to `BusinessException(EmailExists)`; refactored `RegisterDevice` to delegate the row insert to `RegisterUser`. |
|
||||
| D-1 | **High** (CLOSED) | A06 | `Azaion.Common.csproj`, `Azaion.Services.csproj` | `Newtonsoft.Json 13.0.1` < patched line for GHSA-5crp-9r3c-p9vr — **bumped to 13.0.4** |
|
||||
| F-4 | Medium | A02 | `Program.cs:158-162` | `/devices` returns plaintext device password (accepted by design — needs `Cache-Control: no-store` and Swagger trim) |
|
||||
| F-5 | Medium | A05 | `ResourceUpdateService.cs:86-97` | `EncryptionMasterKey` validated lazily on first call instead of at startup |
|
||||
| F-6 | Medium | A05 | `Dockerfile:1,20-25` | API container runs as root |
|
||||
| F-7 | Medium | A02 / A07 | `Security.cs:11-12` | SHA-384 password hashing without per-user salt or KDF (pre-existing) |
|
||||
| F-8 | Medium | A04 / A07 | `Program.cs:137-143` | No rate limiting on `/login` (pre-existing) |
|
||||
| F-9 | Low | A05 | `LoginRequest.cs`, `SetUserQueueOffsetsRequest.cs` | DTOs lack `AbstractValidator<T>` |
|
||||
| F-10 | Low | — | `docker-compose.test.yml`, `appsettings.test.json` | Hardcoded credentials/JWT secret in test fixtures (test-only, accepted) |
|
||||
| F-11 | Low | A05 | `env/db/01_permissions.sql:2,7,12` | Placeholder DB passwords as setup template — needs header comment |
|
||||
| F-12 | Low | A09 | `ResourcesService.cs:63` | Unstructured `LogInformation($"...")` defeats Serilog property capture |
|
||||
| F-13 | Low | A02 | `Program.cs` | No HTTPS enforcement / HSTS in code (assumed at reverse proxy) |
|
||||
|
||||
Detailed evidence and remediation steps for each finding are in `static_analysis.md` (F-N) and `dependency_scan.md` (D-N).
|
||||
|
||||
## Cycle-1 Specific Verdict
|
||||
|
||||
The four cycle-1 tasks introduced one **new** High finding (F-1) and amplified one pre-existing High (F-3). Both were closed before deploy:
|
||||
|
||||
- **F-1 (resolved by revert)**: `/get-update` and `/resources/publish` are deleted; the entire OTA feature (AZ-183) is reverted. The user assessment was that the feature is itself a leftover from the installer-shipping era and is no longer needed in the target architecture (browser-only SaaS + fTPM-secured Jetsons).
|
||||
- **F-3 (resolved)**: `RegisterDevice` now delegates to `RegisterUser`; `users.email` has a UNIQUE INDEX; `RegisterUser` translates UNIQUE-violation to `EmailExists`. The duplicate-row race is closed atomically.
|
||||
- AZ-513 (`/classes` CRUD) and AZ-197 (hardware removal) introduce no new security findings. AZ-197 closes the prior `Hardware Fingerprint Binding` section of `security_approach.md` — the corresponding code, error codes, and DTOs are all gone.
|
||||
|
||||
## Dependency Vulnerabilities
|
||||
|
||||
| Package | CVE / Advisory | Severity | Status |
|
||||
|---------|---------------|----------|--------|
|
||||
| `Newtonsoft.Json` 13.0.1 → 13.0.4 | GHSA-5crp-9r3c-p9vr (DoS via deeply nested JSON) | High | **Closed in this audit** |
|
||||
|
||||
`dotnet list package --vulnerable` returns clean for all five projects against the configured NuGet feeds. Three deprecated-but-not-vulnerable packages are tracked as forward-looking hygiene items in `dependency_scan.md` (`FluentValidation.AspNetCore` 11.3.0, `System.IdentityModel.Tokens.Jwt` 7.1.2, `xunit` 2.x).
|
||||
|
||||
## Recommendations
|
||||
|
||||
### Closed in this audit
|
||||
- **F-1**: OTA feature reverted in full (see "Findings" above).
|
||||
- **F-3**: UNIQUE INDEX added; `RegisterUser`/`RegisterDevice` consolidated; UNIQUE-violation translated to `EmailExists`.
|
||||
- **D-1**: `Newtonsoft.Json` bumped to 13.0.4.
|
||||
|
||||
### Immediate (Critical / High — open)
|
||||
|
||||
1. **F-2** (deferred): Add `dataFolder` validation (`[A-Za-z0-9_-]+` only, plus `Path.GetFullPath(combined).StartsWith(...)` post-check) in `ResourcesService.GetResourceFolder`. Pre-existing finding; file as a separate ticket.
|
||||
|
||||
### Short-term (Medium)
|
||||
|
||||
4. **F-4**: Add `Cache-Control: no-store, no-cache` to the `/devices` response and document the operator runbook entry (no body logging at the reverse proxy).
|
||||
5. **F-5**: Validate `ResourcesConfig.EncryptionMasterKey` at startup in non-Development environments.
|
||||
6. **F-6**: Add `USER app` to the final stage of `Dockerfile`; verify `Content/` and `logs/` are writable to `app` (UID 1654).
|
||||
7. **F-7**: Migrate password hashing to Argon2id with per-user salt (rolling rehash on next login).
|
||||
8. **F-8**: Enable ASP.NET Core 10 rate limiter on `/login` (e.g., 10 req/IP/min).
|
||||
|
||||
### Long-term (Low / hardening)
|
||||
|
||||
9. **F-9**: Add validators for `LoginRequest`, `SetUserQueueOffsetsRequest`.
|
||||
10. **F-11**: Add a header comment to `env/db/01_permissions.sql` flagging it as a template, or rename to `*.example.sql`.
|
||||
11. **F-12**: Convert the one unstructured log line in `ResourcesService.SaveResource` to structured form.
|
||||
12. **F-13**: Document the upstream HTTPS / HSTS / security-header chain in a deployment runbook; consider `app.UseHsts()` once the chain is documented.
|
||||
13. **CI gate**: introduce a build that runs `dotnet build && dotnet test && dotnet list package --vulnerable` and fails the pipeline on any vulnerability finding.
|
||||
|
||||
## Verdict Logic
|
||||
|
||||
- **PASS_WITH_WARNINGS** because exactly one pre-existing **High** finding remains open (F-2 path traversal). The two cycle-1-attributable Highs (F-1 regression, F-3 amplified) and the dependency High (D-1) were all closed during the audit.
|
||||
|
||||
## Tracker Follow-Ups
|
||||
|
||||
The following items should be filed as separate Jira tasks in the AZ project (per `.cursor/rules/tracker.mdc`). Do not write to the tracker as part of this audit — surface them to the user for prioritization first.
|
||||
|
||||
| Ticket | Title | Points |
|
||||
|--------|-------|--------|
|
||||
| [AZ-516](https://denyspopov.atlassian.net/browse/AZ-516) | F-2: Sanitize `dataFolder` route segment to prevent path traversal | 3 |
|
||||
| [AZ-517](https://denyspopov.atlassian.net/browse/AZ-517) | F-4: Harden `/devices` response (Cache-Control, runbook) | 2 |
|
||||
| [AZ-518](https://denyspopov.atlassian.net/browse/AZ-518) | F-6: Run admin API container as non-root | 2 |
|
||||
| [AZ-519](https://denyspopov.atlassian.net/browse/AZ-519) | F-7: Migrate password hashing to Argon2id with per-user salt | 5 |
|
||||
| [AZ-520](https://denyspopov.atlassian.net/browse/AZ-520) | F-8: Add rate limiting to `/login` endpoint | 2 |
|
||||
| [AZ-521](https://denyspopov.atlassian.net/browse/AZ-521) | Low-severity security hygiene bundle (F-9, F-11, F-12, F-13) | 3 |
|
||||
|
||||
**Closed in cycle 1 (no ticket needed)**:
|
||||
- F-1 — OTA feature deleted end-to-end (AZ-183 reverted).
|
||||
- F-3 — UNIQUE INDEX migration `env/db/06_users_email_unique.sql` + `RegisterUser`/`RegisterDevice` consolidation.
|
||||
- D-1 — `Newtonsoft.Json` bumped to 13.0.4.
|
||||
- F-5 — `EncryptionMasterKey` config field deleted along with the OTA feature; the lazy-validation surface no longer exists.
|
||||
@@ -0,0 +1,146 @@
|
||||
# Static Analysis (SAST)
|
||||
|
||||
**Date**: 2026-05-13
|
||||
**Method**: targeted code review of the cycle-1 surface (`/devices`, `/classes` CRUD, `/get-update`, `/resources/publish`) plus regression sweep over the pre-existing endpoints `/login`, `/users/*`, `/resources/*`. No automated SAST tool was run — all findings are manually identified with file/line evidence.
|
||||
|
||||
**Post-audit status (2026-05-13)**: F-1 closed via revert of AZ-183 (OTA feature deleted end-to-end); F-3 closed via UNIQUE INDEX `users_email_uidx` + `RegisterUser`/`RegisterDevice` consolidation; F-5 closed automatically as a consequence of F-1; D-1 closed via `Newtonsoft.Json` 13.0.4 bump. F-2 (path traversal) remains open — pre-existing, deferred to a separate ticket.
|
||||
|
||||
## Findings
|
||||
|
||||
### F-1: `/get-update` exposes per-resource plaintext `EncryptionKey` to any authenticated caller (HIGH — cycle-1 regression — **CLOSED via revert**)
|
||||
|
||||
- **Severity**: High
|
||||
- **Status**: **CLOSED** (2026-05-13) — the entire OTA feature was deleted: `/get-update` and `/resources/publish` endpoints, `IResourceUpdateService` / `ResourceUpdateService` / `ResourceColumnEncryption`, the `Resource` entity, the `resources` table, the `apiUploaderPolicy` policy, the `ResourcesConfig.EncryptionMasterKey` config field, and the `e2e/Azaion.E2E/Tests/ResourceUpdateTests.cs` test class. AZ-183 is reverted; the OTA delivery model is itself obsolete in the target architecture (browser-only SaaS + fTPM-secured Jetsons).
|
||||
- **Category**: Broken Access Control / Cryptographic Failures
|
||||
- **Original locations** (now deleted):
|
||||
- `Azaion.AdminApi/Program.cs:301-312` — endpoint registration used `.RequireAuthorization()` (any logged-in user).
|
||||
- `Azaion.Services/ResourceUpdateService.cs:39-48` — every `ResourceUpdateItem` returned to the caller contained `EncryptionKey = ResourceColumnEncryption.Decrypt(resource.EncryptionKey, MasterKey)`.
|
||||
- **Description**: The `resources.encryption_key` column was encrypted at rest with `ResourcesConfig.EncryptionMasterKey` precisely to mitigate DB compromise. But the application API decrypted and serialized that key into the HTTP response for any caller holding a valid JWT. A low-privilege user could submit `POST /get-update {Architecture, DevStage, CurrentVersions: {}}` and receive — for every published resource — `(CdnUrl, Sha256, EncryptionKey)`. With `EncryptionKey + CdnUrl`, the attacker could pull the encrypted blob from the CDN and decrypt it locally.
|
||||
- **Impact (had it shipped)**: Confidentiality of every published resource (firmware, model weights, installer payloads) reduced to "any authenticated session".
|
||||
- **Resolution rationale**: rather than tightening the policy or filtering the response, the user assessment is that the OTA delivery model itself is no longer needed — the suite is now installed/updated through the browser. Removing the surface eliminates the vulnerability and reduces the attack surface; F-5 (lazy-loaded `EncryptionMasterKey`) is also closed automatically.
|
||||
|
||||
### F-2: Path traversal via `dataFolder` route segment (HIGH — pre-existing, re-flagged)
|
||||
|
||||
- **Severity**: High
|
||||
- **Category**: Broken Access Control / Injection (Path)
|
||||
- **Locations**:
|
||||
- `Azaion.Services/ResourcesService.cs:20-25` — `Path.Combine(ResourcesFolder, dataFolder)` accepts `..` and absolute paths without validation.
|
||||
- Consumed by `Program.cs:201, 213, 219, 224` — `/resources/{dataFolder?}` (any-auth upload), `/resources/list/{dataFolder?}` (any-auth read), `/resources/clear/{dataFolder?}` (admin), `/resources/get/{dataFolder?}` (any-auth read).
|
||||
- **Description**: `Path.Combine("Content", "../../etc")` resolves to `etc`, escaping the configured root. A non-admin caller can:
|
||||
- List arbitrary directories via `/resources/list/../../`.
|
||||
- Read encrypted contents of arbitrary files via `/resources/get/../../<path>` provided they know a filename.
|
||||
- Write into arbitrary directories the process can write to via `/resources/<traversal>` upload.
|
||||
- **Impact**: Server-side file disclosure and arbitrary file write bounded by process privileges. Pre-existing — already noted in `_docs/00_problem/security_approach.md` "Known Security Observations" #3 — but the cycle-1 audit re-confirms it is unmitigated.
|
||||
- **Remediation**:
|
||||
- Sanitize `dataFolder`: reject any value containing `..`, `/`, `\`, or starting with a drive letter; alternatively, allow only `[A-Za-z0-9_-]+` segments.
|
||||
- Verify that the resolved absolute path starts with the resolved `ResourcesFolder` absolute path — `Path.GetFullPath(combined).StartsWith(Path.GetFullPath(root))` — and reject otherwise.
|
||||
|
||||
### F-3: `users.email` lacks a UNIQUE constraint — race in both `RegisterUser` and (cycle-1) `RegisterDevice` (HIGH — **CLOSED**)
|
||||
|
||||
- **Severity**: High
|
||||
- **Status**: **CLOSED** (2026-05-13).
|
||||
- **Resolution**:
|
||||
- Added migration `env/db/06_users_email_unique.sql` containing `CREATE UNIQUE INDEX IF NOT EXISTS users_email_uidx ON public.users (email);`. The migration is wired into `e2e/db-init/00_run_all.sh`.
|
||||
- `Azaion.Services/UserService.cs` — `RegisterUser` no longer check-then-inserts. It catches `Npgsql.PostgresException` with `SqlState == PostgresErrorCodes.UniqueViolation` (23505) and rethrows as `BusinessException(EmailExists)`. The race is closed atomically by the index.
|
||||
- `RegisterDevice` was refactored to delegate the row insert to `RegisterUser` (the user's explicit guidance: "reuse the code in the implementation RegisterDevice -> should call RegisterUser"). Two concurrent provisioning calls that race on the same serial now hit the UNIQUE INDEX and surface `BusinessException(EmailExists)`; the caller can retry.
|
||||
- **Residual risk**: a Postgres sequence for device serials (`device_serial_seq`) would also remove the serial-allocation race window and avoid the retry. Out of scope for this audit fix; can be added as a follow-up.
|
||||
|
||||
### F-4: `/devices` returns plaintext device password in the JSON body (MEDIUM — accepted by design, hardening required)
|
||||
|
||||
- **Severity**: Medium
|
||||
- **Category**: Cryptographic Failures / Data Exposure
|
||||
- **Locations**:
|
||||
- `Azaion.AdminApi/Program.cs:158-162`
|
||||
- `Azaion.Services/UserService.cs:84-89` (assembles `RegisterDeviceResponse`)
|
||||
- **Description**: The endpoint deliberately returns the plaintext device password "exactly once" so the provisioning script can write it to `device.conf`. ApiAdmin-only, so abuse blast radius is bounded — but the password is reachable via:
|
||||
- Reverse-proxy access logs that capture response bodies
|
||||
- Browser DevTools / network history when triggered from the admin UI
|
||||
- Swagger UI's "Try it out" response panel in any environment where Swagger is exposed (today: `IsDevelopment()` only — verified).
|
||||
- **Impact**: Credentials may persist in unintended log sinks beyond their intended one-shot consumption.
|
||||
- **Remediation**:
|
||||
- Set response headers `Cache-Control: no-store, no-cache`, `Pragma: no-cache` on this endpoint specifically.
|
||||
- Document an SRE runbook entry: do NOT enable response-body logging on the reverse proxy for `POST /devices`.
|
||||
- Optional: add an `X-One-Shot-Credential: true` header so log scrubbers can match-and-mask.
|
||||
|
||||
### F-5: `EncryptionMasterKey` validation is lazy — first failing request, not startup (MEDIUM — **CLOSED**)
|
||||
|
||||
- **Severity**: Medium
|
||||
- **Status**: **CLOSED** (2026-05-13) — `ResourcesConfig.EncryptionMasterKey` and the `ResourceUpdateService.MasterKey` getter were both deleted along with the OTA feature (see F-1). The lazy-validation surface no longer exists; `appsettings.json` and `docker-compose.test.yml` no longer reference the field.
|
||||
|
||||
### F-6: API container runs as root (MEDIUM)
|
||||
|
||||
- **Severity**: Medium
|
||||
- **Category**: Security Misconfiguration
|
||||
- **Location**: `Dockerfile:1, 20-25`
|
||||
- **Description**: `mcr.microsoft.com/dotnet/aspnet:10.0` defaults to `root`. There is no `USER` directive after `FROM base AS final`. CIS Docker Benchmark §4.1 calls this out: a process running as root inside the container has more privileges than necessary, and a container escape (CVE-2024-21626 class) becomes a root-on-host exploit.
|
||||
- **Impact**: Defense-in-depth weakness. No specific exploit, but failure mode is severe.
|
||||
- **Remediation**: Add `USER app` to the final stage (the .NET 10 base image already provisions a non-root `app` user, UID 1654). The Content/log directory permissions need to be checked once the change is made.
|
||||
|
||||
### F-7: SHA-384 password hashing without per-user salt or KDF (MEDIUM — pre-existing)
|
||||
|
||||
- **Severity**: Medium
|
||||
- **Category**: Cryptographic Failures
|
||||
- **Locations**:
|
||||
- `Azaion.Services/Security.cs:11-12` — `ToHash()` hashes raw UTF-8 bytes with SHA-384.
|
||||
- `Azaion.Services/UserService.cs:44, 110, 78` — used for `RegisterUser`, `ValidateUser`, `RegisterDevice` (cycle-1 added the `RegisterDevice` call site).
|
||||
- **Description**: SHA-384 is a fast cryptographic hash, not a password-hashing algorithm. No per-user salt, no work factor, no memory hardness. A leaked `password_hash` column lets an offline attacker grind ~10⁹ candidates per second per GPU.
|
||||
- **Impact**: Database leak directly compromises all user passwords in tractable time.
|
||||
- **Remediation**: Migrate to Argon2id (e.g., `Konscious.Security.Cryptography.Argon2`) or bcrypt (`BCrypt.Net-Next`) with per-user salt. Two-phase rollout: rehash on next successful login until the SHA-384 column is empty, then drop it.
|
||||
|
||||
### F-8: No rate limiting on `/login` (MEDIUM — pre-existing)
|
||||
|
||||
- **Severity**: Medium
|
||||
- **Category**: Auth Failures
|
||||
- **Location**: `Azaion.AdminApi/Program.cs:137-143`
|
||||
- **Description**: Combined with F-7, an attacker who can reach `/login` can brute-force credentials. ASP.NET Core 10 ships `AddRateLimiter()` out of the box.
|
||||
- **Remediation**: Add a fixed-window or sliding-window limiter scoped to `/login` (e.g., 10 requests / IP / minute, with exponential backoff).
|
||||
|
||||
### F-9: `LoginRequest`, `SetUserQueueOffsetsRequest` lack server-side validation (LOW — pre-existing)
|
||||
|
||||
- **Severity**: Low
|
||||
- **Category**: Security Misconfiguration
|
||||
- **Locations**:
|
||||
- `Azaion.Common/Requests/LoginRequest.cs` — no validator class
|
||||
- `Azaion.Common/Requests/SetUserQueueOffsetsRequest.cs` — no validator class
|
||||
- **Description**: Other request DTOs use `AbstractValidator<T>` (`RegisterUserValidator`, `GetUpdateValidator`, etc.). These two are unguarded — `LoginRequest` accepts any-length email/password, `SetUserQueueOffsetsRequest` accepts any email shape and any offsets payload.
|
||||
- **Remediation**: Add validators with `EmailAddress()` + `MinimumLength(12)` (matching `RegisterUserValidator`) and bounds checks for `Offsets`.
|
||||
|
||||
### F-10: Hardcoded credentials and JWT secret in test fixtures (LOW — accepted)
|
||||
|
||||
- **Severity**: Low
|
||||
- **Category**: Hardcoded Credentials
|
||||
- **Locations**:
|
||||
- `docker-compose.test.yml:31-33, 37` — DB credentials, JWT secret, encryption master key as compose env vars.
|
||||
- `e2e/Azaion.E2E/appsettings.test.json:4-7` — `AdminPassword`, `UploaderPassword`, `JwtSecret`.
|
||||
- **Description**: These are e2e-only and consistent across the harness. They are NOT used in production builds. Flagged here for visibility only — they MUST NEVER drift into the prod compose / appsettings.
|
||||
- **Remediation**: Add a CI guard: fail the pipeline if any of these literals appear in `Azaion.AdminApi/appsettings.json` or `Azaion.AdminApi/appsettings.Production.json`.
|
||||
|
||||
### F-11: `env/db/01_permissions.sql` ships placeholder DB passwords as a setup template (LOW)
|
||||
|
||||
- **Severity**: Low
|
||||
- **Category**: Hardcoded Credentials (template / docs)
|
||||
- **Location**: `env/db/01_permissions.sql:2, 7, 12` — `superadmin-pass`, `admin-pass`, `readonly-pass`.
|
||||
- **Description**: The file is the operator setup template. The e2e harness immediately overrides these with `test_password` (`e2e/db-init/99_test_seed.sql:1-2`), so the placeholders never reach a runtime. But the file lives at `env/db/` with no header comment marking it template-only.
|
||||
- **Remediation**: Add a top-of-file comment `-- TEMPLATE: replace placeholder passwords before applying to any environment.` Consider renaming to `01_permissions.example.sql`.
|
||||
|
||||
### F-12: Unstructured logging in `ResourcesService.SaveResource` (LOW)
|
||||
|
||||
- **Severity**: Low
|
||||
- **Category**: Logging Failures (operational)
|
||||
- **Location**: `Azaion.Services/ResourcesService.cs:63` — `logger.LogInformation($"Resource {data.FileName} Saved Successfully")`.
|
||||
- **Description**: String interpolation defeats Serilog's structured property capture; the `FileName` is not searchable as a field. Not a security issue, but flagged because the security-event-logging principle (audit trail) requires structured fields.
|
||||
- **Remediation**: `logger.LogInformation("Resource {FileName} saved successfully", data.FileName);`
|
||||
|
||||
### F-13: No HTTPS enforcement in application code (LOW — pre-existing, design)
|
||||
|
||||
- **Severity**: Low
|
||||
- **Category**: Cryptographic Failures
|
||||
- **Location**: `Azaion.AdminApi/Program.cs` — no `app.UseHttpsRedirection()`, no `Hsts`.
|
||||
- **Description**: HTTPS is assumed at the reverse proxy. Acceptable design choice if and only if the reverse proxy and its config are part of the secure boundary.
|
||||
- **Remediation**: Document the assumption in a deployment runbook; consider `UseHsts()` when the upstream chain terminates TLS.
|
||||
|
||||
## Self-verification
|
||||
|
||||
- [x] All source directories scanned (`Azaion.AdminApi/`, `Azaion.Services/`, `Azaion.Common/`, `env/db/`, `Dockerfile`)
|
||||
- [x] Each finding has file path and (where relevant) line numbers
|
||||
- [x] No false positives from test files or comments — test-fixture credentials (F-10) are explicitly framed as accepted-risk
|
||||
Reference in New Issue
Block a user