91 lines
5.5 KiB
Markdown
91 lines
5.5 KiB
Markdown
# Refinement Phase 5 — Security & config hygiene (deployment blockers)
|
||
|
||
> **Mission:** remove the committed secrets and dev-grade defaults that block any non-local deployment. None of
|
||
> this changes behavior, but two items are **live credential leaks sitting in git today**. Do the credential
|
||
> rotation (5.1) *now*, independently of everything else.
|
||
>
|
||
> **Track:** backend (config/security) · **Depends on:** nothing (can run in parallel with Phases 0–4) ·
|
||
> **Unlocks:** any real/shared-environment deployment
|
||
> **Before you start, read [../../phases/_shared/agent-operating-rules.md](../../phases/_shared/agent-operating-rules.md).**
|
||
|
||
## 1. Context
|
||
|
||
This phase **is** the already-written server audit's **post-phase-1**. It is reproduced here as a refinement
|
||
phase for sequencing; the full evidence (file/line for every item) and the exact fixes live in
|
||
**[../server/post-phase-backend-plan.md](../server/post-phase-backend-plan.md) § post-phase-1**. Read that
|
||
section — it is the spec. This file only orders and frames it.
|
||
|
||
The findings, in short (all verified in code, dated 2026-07-10):
|
||
|
||
1. **Committed `sa` connection string** — a real remote SQL Server (public IP `87.107.152.16`, plaintext
|
||
password) in `appsettings.json` **and** `appsettings.Development.json` (byte-identical), for both the app DB
|
||
and the log DB. Anyone with repo access owns the database. Violates root `CLAUDE.md` agreement #6.
|
||
2. **Placeholder JWE signing/encryption + PII field-encryption keys** (`local-dev-…-change-me`), a ~7-day
|
||
access-token lifetime (`ExpirationMinutes: 10000`), `RequireHttpsMetadata = false`, and stub
|
||
`Issuer`/`Audience` (`MyWebsite`).
|
||
3. **Seeded `admin` / `qw123321`** full-admin user on every non-Testing boot.
|
||
4. **Auto-seeded active sandbox ZarinPal gateway** on every boot (a production DB would silently hold an active
|
||
sandbox money gateway).
|
||
5. **Kestrel HTTP/2-only default** (breaks non-TLS HTTP/1.1 hops).
|
||
6. **Rate limiter not proxy-aware** (`RemoteIpAddress`, no `ForwardedHeaders`) → behind a proxy every client
|
||
shares one bucket; plus the two payment webhooks use mismatched rate policies.
|
||
|
||
## 2. Required reading
|
||
|
||
- **[../server/post-phase-backend-plan.md](../server/post-phase-backend-plan.md)** § post-phase-1 (items
|
||
1.1–1.6) — the authoritative, file/line-cited spec.
|
||
- **[../server/runtime-services.md](../server/runtime-services.md)** § 1–3 (SQL Server + reverse-proxy notes).
|
||
- [Phase 0](refinement-phase-0-bring-up.md) — it already introduced a secret-free local DB default; this phase
|
||
finishes the rotation and the deployed-environment story.
|
||
|
||
## 3. Scope — deliver plan items 1.1–1.6
|
||
|
||
- **5.1 (do first, today) — rotate & remove the committed `sa` connection strings.** Assume compromised: rotate
|
||
the password, create a least-privilege app login, move both connection strings to user-secrets (dev) / env
|
||
vars (deploy), commit only a placeholder, and add a secret-scanning pre-commit hook. Consider history
|
||
scrubbing (`git filter-repo`).
|
||
- **5.2 — replace the placeholder JWE + field-encryption keys** with per-environment secrets; set a sane
|
||
access-token lifetime (≤ 60 min; refresh already exists); `RequireHttpsMetadata = true` outside Development;
|
||
real `Issuer`/`Audience`. (Rotating the field key needs a re-encryption migration — do it before real PII
|
||
exists.)
|
||
- **5.3 — environment-gate the seeded admin** (read bootstrap creds from config; Development-only; force a
|
||
password change on first login).
|
||
- **5.4 — environment-gate the sandbox gateway seed** (Development/Testing only, or seed `is_active = false`).
|
||
- **5.5 — fix the Kestrel HTTP/2-only default** (`Http1AndHttp2`; give gRPC its own endpoint if kept).
|
||
- **5.6 — make rate limiting proxy-aware** (`ForwardedHeaders` trusting the known proxy; partition on the
|
||
resolved client IP) and pick one deliberate webhook rate policy.
|
||
|
||
## 4. Mocks & seams
|
||
|
||
None. Config/security only.
|
||
|
||
## 5. Critical rules
|
||
|
||
- **5.1 is urgent and independent** — it doesn't wait for this phase's slot; the credential is leaking now.
|
||
- Rotating the field-encryption key **invalidates existing dev-DB ciphertext** — acceptable pre-launch, but do
|
||
it before real PII exists and plan the re-encryption migration if any real data exists.
|
||
- Don't break local dev: Development keeps working defaults (via user-secrets/env), only the *committed* values
|
||
become placeholders.
|
||
|
||
## 6. Definition of Done
|
||
|
||
- [ ] No working secret remains in any committed file (connection strings, JWE keys, field keys, admin
|
||
password, gateway creds) — verified by a secret scan.
|
||
- [ ] Access-token lifetime sane; `RequireHttpsMetadata` gated by environment; real issuer/audience.
|
||
- [ ] Admin + sandbox-gateway seeds are Development-gated (or config-driven).
|
||
- [ ] Kestrel default is `Http1AndHttp2`; rate limiter honors forwarded headers; webhooks use one deliberate
|
||
policy. `dotnet build`/`dotnet test` green.
|
||
|
||
## 7. How to test
|
||
|
||
- Fresh clone with no user-secrets → the app **fails fast with a clear "missing connection string" error**
|
||
(not a silent connect to a leaked remote). With user-secrets/env set → boots normally.
|
||
- Confirm no `admin`/`qw123321` and no active sandbox gateway appear in a non-Development boot.
|
||
- Behind a reverse proxy, distinct client IPs get distinct rate-limit buckets.
|
||
|
||
## 8. Hand off & document
|
||
|
||
- Update `server/CLAUDE.md` (startup wiring / identity / rate-limiting) to reflect the config-driven secrets and
|
||
the forwarded-headers middleware. Record the credential rotation in the security log / handoff. Save a memory
|
||
note that the committed `sa` string was rotated and externalized.
|