Sitelet https://github.com/arb-rajab/flexcatalog/blob/main/docs/project-memory/risk.md
Skip to content

risk.md

Latest commit

 

History

History
24 lines (21 loc) · 7 KB

risk.md

File metadata and controls

24 lines (21 loc) · 7 KB
Raw

Risk Register

# Risk Likelihood Impact Mitigation / status
R1 Cross-tenant data leakage (one tenant sees another's data) Low (structurally mitigated) Critical Repository-enforced tenant scoping (ADR 0001) + dedicated integration test suite (TenantIsolationTests). Residual risk: a future contributor adds a new tenant-scoped collection without following the same pattern -- mitigated by documenting the convention explicitly in ADR 0001, not by a compiler check.
R2 JWT secret reused from the committed development placeholder in a real deployment Low (structurally mitigated) Critical Closed. App fails to start with an empty secret, and now also refuses to start in Production if Jwt:Secret equals the known dev placeholder value (JwtSecretGuard.EnsureNotPlaceholder, called from Program.cs; pinned by JwtSecretGuardTests). Residual risk: an operator could still choose a different weak secret -- that's inherently undetectable and remains a documented operational-discipline requirement in security.md.
R3 Compromised JWT is valid until natural expiry (no revocation) Low-Medium Medium Deliberately deferred, not half-shipped -- see backlog.md item 3 for the explicit reasoning recorded this session. Token lifetime capped at 60 minutes by default; no revocation list exists. Acceptable for this system's scope; real production use would need a blocklist or move to short-lived tokens + refresh tokens.
R4 Brute-force login attempts (no rate limiting) Low (mitigated) Medium Closed. POST /api/auth/login now enforces a fixed-window rate limit (5 attempts / 60s per client IP, HTTP 429 on excess, no queueing) via ASP.NET Core's built-in rate limiter (LoginRateLimiting, wired in Program.cs/AuthEndpoints.cs). Pinned by LoginRateLimitingTests (unit) and AuthEndpointsTests.Login_ExceedingRateLimit_Returns429ThenRecoversNextWindow (integration). BCrypt's cost factor (12) remains a secondary slow-down. Residual risk: distributed brute force across many source IPs isn't mitigated by per-IP partitioning -- would need a global/account-based secondary limit if that threat model matters.
R5 Faceted search attribute-filter keys used as a NoSQL injection vector Low (mitigated) High Keys validated against a strict allowlist regex before being used to build a Mongo field path; values are always passed as typed BSON, never string-concatenated. Covered by a dedicated test (BuildMatchDocument_RejectsUnsafeAttributeKeys).
R6 Compound wildcard attribute index (ADR 0002) has higher write-time index-maintenance cost than a handful of targeted indexes Medium Low-Medium at current scale Accepted tradeoff for schema flexibility across categories; revisit if/when write throughput on products becomes a measured bottleneck -- at that point, targeted per-field indexes for the highest-traffic attributes could supplement or replace the wildcard index.
R7 Faceted search facet counts are computed net of the full filter, not independently per facet (ADR 0004) N/A (documented behavior, not a defect) Low Closed for the category facet and attribute facets (brand/sizes/colors/author) -- each is now computed net of every filter except its own; see ADR 0004's "Resolved" section. The price-range facet remains a deliberate exception (still net of the full filter, including price) -- a continuous range indicator, not a togglable option list, so independent-branch semantics don't apply the same way.
R8 Single shared MongoDB database for all tenants means one tenant's very large or very hot dataset can affect others ("noisy neighbor") Low at demo scale Medium if it ever ran at real scale ADR 0001 documents database-per-tenant as the scaling path if this ever became a real concern; not implemented because it would be premature for this system's actual scale and purpose.
R9 No centralized/structured logging or tracing High (true today) Low for a demo, Medium for real ops Standard ASP.NET Core console logging only. Acceptable for a portfolio demo; tracked in backlog.md as what a production hardening pass would add first.
R10 Integration test suite (Testcontainers) could not be executed inside the development sandbox due to an egress policy blocking Docker Hub N/A (environment limitation, not a product risk) N/A Verified instead by GitHub Actions CI, which has unrestricted internet access; see testing.md and ops.md for exact detail and handoff.md for the confirmed CI result.
R11 Domain events (ADR 0006) are at-most-once and dropped silently if NATS or FlexCatalog.InventoryProjector is unreachable/down N/A (accepted, documented behavior, not a defect) Low (the projection is a convenience read-model nothing else depends on) Deliberate tradeoff: requirement was that publishing must never be able to fail or slow the primary MongoDB write, which rules out at-least-once/retry semantics on the request path. JetStream is the documented upgrade path if a future consumer needs durability (backlog.md #15).
R12 main has no branch protection rule at all -- CI passing on a PR is advisory, not enforced; a broken PR can merge High (true today) High (defeats the purpose of the entire CI suite -- see below) Open, needs a human with admin access. Confirmed via GET /repos/arb-rajab/flexcatalog/branches (the main entry's protected field is false) that no branch protection rule exists on main, the repo's actual default branch. .github/workflows/ci.yml has two PR-triggered jobs, both unconditional (no path filters, so requiring either carries no risk of blocking unrelated PRs): build-and-test (format check, build, unit tests, integration tests -- the correctness/regression-bearing job) and docker-build (verifies all three Dockerfiles still build). Neither is a required status check today. Session tooling could read this (branches API, workflow file) but had no branch-protection write path (no gh/curl fallback available this session, no branch-protection endpoint in the GitHub MCP server provided), so the fix could not be applied directly and is documented here instead. Exact fix for a human with admin rights: in GitHub, Settings -> Branches -> Add branch protection rule (or Settings -> Rules -> Rulesets) for main, enable "Require status checks to pass before merging", and add build-and-test and docker-build (the two job names in .github/workflows/ci.yml) as required checks. Equivalent via API: PUT /repos/arb-rajab/flexcatalog/branches/main/protection with required_status_checks.contexts (or checks) including build-and-test and docker-build, enforce_admins per team preference, and required_pull_request_reviews per team preference.

How this list is used

Reviewed whenever a new feature touches auth, tenant scoping, or search query-building -- those three areas carry the risks with the highest impact (R1, R2, R5). Anything added to backlog.md that closes one of these rows should update the row's status here rather than leaving it stale.