feat: Add comprehensive observability, security, and simulation improvements - #366
Merged
Merged
Conversation
|
@tecch-wiz Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
Contributor
CI Checks Failed — Action NeededThe continuous integration checks on this pull request are failing. I can't merge until all required checks pass. Failing checks:
What to do: review the failing checks in the "Checks" tab above, fix the issues in your code, and push the changes. |
Contributor
Merge Conflict — Action NeededThis pull request has merge conflicts with the base branch ( What to do: update your branch by merging or rebasing against |
1 similar comment
Contributor
Merge Conflict — Action NeededThis pull request has merge conflicts with the base branch ( What to do: update your branch by merging or rebasing against |
Implement comprehensive risk scoring service with data persistence for compliance tracking, complete OpenAPI documentation coverage, and add integration tests for core infrastructure components. Risk Scoring Service (ASTROIDX556#213): - Add RiskRepository for assessment record persistence and historical analysis - Update RiskService to persist assessment records via repository - Add getHistory() and getStatistics() methods for risk analytics - Add RiskAssessment model to Prisma schema with proper indexes - Create database migration for risk assessments table - Add comprehensive integration tests for RiskRepository Response Envelope Interceptor (ASTROIDX556#216): - Verify ResponseInterceptor is registered globally in app.module.ts - Add integration tests for response wrapping and pagination handling - Test requestId header handling and null data scenarios Swagger Documentation (ASTROIDX556#218): - Add @ApiProperty decorators to admin DLQ and queue management DTOs - Add @ApiProperty decorators to audit export DTOs - Add @ApiProperty decorators to risk assessment DTOs - Update risk controller to use DTO for request body documentation - Complete OpenAPI documentation coverage across all domain modules Zod Validation Pipe (ASTROIDX556#220): - Verify ZodValidationPipe supports custom error formatting - Add integration tests for validation scenarios and error handling - Test optional fields, nested objects, arrays, and custom messages
Add organization relation field to RiskAssessment model to fix Prisma schema validation error. Update migration to add foreign key constraint separately for proper schema validation.
Fix TypeScript errors in test files by: - Converting async tests to use toPromise() instead of callbacks - Adding proper type assertions for exception details - Adding RiskRepository to RiskService constructor - Using type assertions for Prisma client methods until migration runs - Adding missing PaginationMeta properties in tests
…tests Add null checks after toPromise() calls to resolve TypeScript 'possibly undefined' errors in response interceptor tests.
Add eslint-disable comments for @typescript-eslint/no-explicit-any where we use type assertions for Prisma client methods until the migration runs and generates the proper types.
Change the test expectation to use expect.objectContaining for the nested createdAt object instead of expect.any(Date) for the entire field, as the Date object is being compared with its actual value.
…rs (ASTROIDX556#356) * feat: merge PR ASTROIDX556#284 — add RiskRepository, RiskAssessment schema, and test files Resolves the merge conflict from PR ASTROIDX556#284 by applying all genuinely new additions (RiskRepository, schema migration, test specs, service updates) while keeping main's already-improved Swagger DTO implementations. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012xRrTYwEy2y8yow4o8iUQ8 * fix: use ZodValidationException in integration spec The pipe throws ZodValidationException (a BadRequestException subclass defined in zod-validation.pipe.ts), not the domain-layer ValidationException. Update all assertions to match the actual thrown type. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012xRrTYwEy2y8yow4o8iUQ8 --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds GET /health/redis, mirroring GET /health/database, so container orchestration and uptime monitors can probe Redis alone instead of only seeing it as one service inside /health/readiness. Reports status, latency and the ping error, with 200/503 semantics, plus unit tests for the up, down and isolation paths.
…IDX556#359) Guard and config behavior for rate limiting is already covered in isolation, but nothing asserted that register/login/refresh actually declare the auth tier. Adds a regression test against the decorator metadata so removing @ThrottleTierDecorator('auth') from a handler fails CI instead of silently dropping back to the looser api limit. Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
) Add orchestrator-grade liveness and readiness probes: - GET /health/live returns 200 whenever the process is running and performs no dependency checks, so a downstream outage never triggers a restart. - GET /health/ready probes the database (SELECT 1) and cache (Redis PING) in parallel, each bounded by a 2s timeout, and returns 200 or 503 with a per-dependency status, latency and error report under `services`. - Both probes are served outside the global API prefix, like /metrics, so probe paths are stable across API versions. Make the health controller usable by load balancers: - Mark it @public(); previously every health route required a JWT. - Exempt it from both named throttler tiers so probes cannot receive 429s. - Exclude it from the audit trail so probes do not write an audit row per request (or attempt to while the database is down). Fix the Redis indicator to probe the shared REDIS_CLIENT built from the validated REDIS_* config. It previously read an undefined REDIS_URL, always probed localhost:6379, kept its own never-closed connection, and could hang while ioredis queued the PING during an outage. Closes ASTROIDX556#351
Compose the per-slice Zod schemas into a single `environmentSchema` and validate `process.env` against it at the start of bootstrap(), before any Nest module is constructed or any connection is opened. - On failure the process prints every failing variable in one message and exits with code 1, instead of surfacing only the first failing slice from inside Nest's module initialization with a stack trace. - Messages are value-free (e.g. "must be one of: ..." rather than Zod's default "received '<value>'") so secrets never reach logs. - In production, reject the publicly known default ENCRYPTION_KEY (whether set explicitly or implied by omission) and a JWT refresh secret that reuses the access secret. Per-slice validation in each registerAs factory is unchanged, so the typed ConfigService namespaces keep their guarantees outside main.ts. Document every variable, its type, default and production rules in docs/configuration.md, and correct the README's list of required variables. Tests keep the docs and .env.example in sync with the schema, and exercise the real main.ts entrypoint to prove missing or malformed variables halt startup before NestFactory.create is called. Closes ASTROIDX556#350
…STROIDX556#367) Closes ASTROIDX556#331 Closes ASTROIDX556#325 Closes ASTROIDX556#334 Closes ASTROIDX556#328 - Inbound webhook receiving endpoint (POST /webhooks/receive) wired to the existing but previously unwired RawBodyMiddleware + WebhookSignatureGuard, with a Zod schema validating the payload shape. - Removed duplicate BullMQ processor consuming the webhooks queue (WebhookWorker), keeping WebhooksProcessor which also records metrics. Deleted dead WebhookDeliveryWorker, never registered as a real processor. - Wired the existing, previously unused SlidingWindowThrottlerGuard onto the new public ingress route for Redis-backed sliding-window rate limiting with standard rate-limit headers. - Added StreamMetricsService: ring-buffer based p95/p99 latency aggregation per stream, exposed through the existing Prometheus registry.
…STROIDX556#371) Add a migration CLI (npm run db:migrate -- <command>) with a production safety guard. The destructive commands, down and reset, are rejected when NODE_ENV=production unless --force is supplied. A blocked run prints a warning to stderr, exits with code 1 and never contacts the database. A forced run is also announced on stderr. down <migration> runs the migration's hand-written down.sql and removes its _prisma_migrations row in a single script, so the history row is only dropped when every rollback statement succeeded. Migration names are validated against the folder on disk before being used in SQL. The guard logic lives in src/database/migration-guard.ts with injected process dependencies so it is fully unit tested. The entry point src/database/migrate.cli.ts is compiled into dist and can run in images without ts-node. Usage is documented in docs/database.md.
…uards (ASTROIDX556#373) Add dedicated specs for RolesGuard, PermissionsGuard and ScopesGuard, including matchScope, and extend the RbacGuard spec. The guards now have 100% statement, branch, function and line coverage. The tests attach real @roles, @RequirePermissions, @RequireScopes and @public metadata to fixture controllers and run them through a real Reflector, so handler-over-class inheritance is exercised rather than mocked. Assertion matrices cover every UserRole, every wildcard shape in matchScope, including nested scopes, and the AND semantics of multi- permission routes. Behaviour pinned by these tests: - OWNER bypasses RolesGuard, and a JWT-authenticated OWNER or ADMIN bypasses ScopesGuard; API-key principals with those roles do not. - PermissionsGuard grants no role-based override and expands no wildcards. - Guests and expired or revoked principals, which the auth strategies leave without request.user, get a 401 on every restricted route. - Stale or differently cased role names are rejected.
… memory (ASTROIDX556#374) Every paginated list endpoint defaults to ORDER BY "createdAt" DESC, but the tables behind the busiest ones only had single-column indexes. Postgres therefore had to fetch all of a tenant's or user's rows, or walk the global createdAt index and filter, before it could return a page. These composite indexes match the actual query shapes: - notifications (userId, createdAt): inbox list - notifications (organizationId, userId, read, createdAt): unread badge, mark-all-read, unread filter (index-only count) - proposals (organizationId, createdAt): approval queue - proposals (organizationId, status, createdAt): pending count, status filter - memory_records (organizationId, createdAt): memory browser - memory_records (agentId, createdAt): per-agent memory timeline Each index is built with CREATE INDEX CONCURRENTLY, so writes are never blocked. Each one lives in its own single-statement migration, because Prisma runs a multi-statement migration as one implicit transaction, where Postgres rejects CONCURRENTLY. The matching @@index entries are added to schema.prisma so migrate dev reports no drift. docs/concurrent-indexes.md records the convention and how to recover from a failed concurrent build.
Add PublicRateLimitGuard, a global guard that applies a per-IP sliding window limit to every unauthenticated route: handlers marked @public() and any route under /<API_PREFIX>/public/. Requests beyond the limit are rejected with 429 Too Many Requests and a Retry-After header. - X-RateLimit-Limit, X-RateLimit-Remaining and X-RateLimit-Reset are set on every limited response - counters live in the shared REDIS_CLIENT via an atomic Lua sliding window script, so all replicas enforce one budget per IP; rejected requests are not recorded - if Redis is unavailable the guard falls back to an in-memory window per instance instead of failing open - thresholds are configurable with PUBLIC_RATE_LIMIT_ENABLED, PUBLIC_RATE_LIMIT_MAX_REQUESTS, PUBLIC_RATE_LIMIT_WINDOW_SECONDS and PUBLIC_RATE_LIMIT_TRUST_PROXY (X-Forwarded-For is ignored by default) - @SkipPublicRateLimit() exempts routes; applied to the network-restricted /metrics scrape endpoint - add store and guard unit tests plus an HTTP burst integration test
…DX556#378) Replace the { success, error: { code, message }, requestId } error envelope with a uniform problem details body served as application/problem+json: { type, title, status, detail, instance, code, requestId, details? } - type is a stable URN per ErrorCode (urn:astroid:problem:<code>); HTTP errors without a dedicated code use about:blank with the reason phrase - title comes from a new ERROR_TITLE map kept exhaustive by the type system; instance is the request path without its query string - code and requestId are kept as extension members so clients can keep switching on the machine-readable code - ZodValidationException now keeps its VALIDATION_ERROR code and field-level details instead of collapsing to BAD_REQUEST - unhandled exceptions still map to a generic 500 INTERNAL_ERROR without leaking internals - update the shared response types and API documentation - rewrite the filter spec and add an HTTP integration test covering validation, authentication, domain, not-found and server errors
* fix: worker error handling + log scrubbing (resolve PR ASTROIDX556#370 conflicts) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012xRrTYwEy2y8yow4o8iUQ8 * test: add job-worker spec and queue-failure-listener scrubbing test Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012xRrTYwEy2y8yow4o8iUQ8 --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
…ASTROIDX556#368 conflicts) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012xRrTYwEy2y8yow4o8iUQ8
…ASTROIDX556#338, ASTROIDX556#339) (ASTROIDX556#369) * perf(analytics): batch dashboard overview queries into one transaction Closes ASTROIDX556#338 AnalyticsService.overview() issued 7 independent queries (counts, spend aggregates, status/risk group-bys) via Promise.all, each its own roundtrip. Batches the 3 counts and 2 aggregates into a single $transaction([...]) call; the 2 groupBy calls stay outside the batch since Prisma's groupBy return type doesn't infer correctly inside a $transaction array. Response shape is unchanged. Existing indexes on organizationId/status/createdAt already cover these queries. * test(audit): cover pagination and sorting edge cases for activity log Closes ASTROIDX556#339 The audit log (this repo's activity log) already supported page/limit/sort/order/filter query params and returned pagination metadata (total, totalPages, hasNext, hasPrev), with limit capped at 100. Adds unit test coverage for the previously-untested list() method: normal pagination, empty results, out-of-bounds pages, invalid sort field fallback, ascending order, and entity filtering. * fix(migrations): resolve colliding timestamp between two merged migrations Migrations 20260928120000_add_agent_contribution_stats_index and 20260928120000_add_notifications_user_created_at_index landed with the same 14-digit timestamp prefix from two separately merged PRs (ASTROIDX556#374, ASTROIDX556#357), which scripts/verify-migrations.sh rejects as a conflict. Bumps the notifications index migration to 20260928120001; both migrations are independent, additive CREATE INDEX statements with no ordering dependency between them, so the rename is safe. * fix(ci): document missing env vars and fix flaky retry.util test Two pre-existing, unrelated-to-this-PR CI failures fixed while unblocking this branch: - docs/configuration.md was missing 7 env vars added by recent merges (DATABASE_SLOW_QUERY_THRESHOLD_MS, DATABASE_CONNECT_RETRY_ATTEMPTS, DATABASE_CONNECT_RETRY_DELAY_MS, PUBLIC_RATE_LIMIT_*), which env.validation.spec.ts asserts against. Documented all 7. - retry.util.spec.ts had 3 tests that create a rejecting promise, advance fake timers with vi.runAllTimersAsync(), then attach the rejection assertion afterward — a race that surfaces as an unhandled rejection under full-suite load (deterministic once >100 files run together). Attaching a no-op .catch() immediately after creating the promise prevents the unhandled state without changing what each test asserts.
…r Transaction Risk Scoring (ASTROIDX556#381)
…utating Operations (ASTROIDX556#384)
…ensitive API Endpoints (ASTROIDX556#383)
Wires the existing migration status checker into app bootstrap so the process halts before accepting traffic when prisma/migrations has pending or failed migrations (DATABASE_MIGRATION_CHECK_MODE=halt, the default; 'warn' logs and continues). Gated behind DATABASE_MIGRATION_CHECK_ENABLED. Adds a db_pool_connections Prometheus gauge (active/idle/waiting) sourced from pg_stat_activity, since Prisma's Rust query engine doesn't expose pool internals through the Node client. Closes ASTROIDX556#335 Closes ASTROIDX556#332
main's build/typecheck/lint/test were already broken before this branch touched anything (confirmed by checking out upstream/main directly). CI enforces these repo-wide, so they block this PR too. Fixed each: - event-names.ts: duplicate object key (TransactionRiskScoringRequested) - throttler.guard.ts: read AuthenticatedUser.sub, a field that doesn't exist on that type (JWT payload field name leaked into the wrong type) - sliding-window-throttler.guard.ts: removed a user-tier rate-limit multiplier keyed on AuthenticatedUser.tier, a field never present anywhere in the auth/user model — dead, unbacked logic. Removed its now-orphaned tests too. - agent.controller.ts: AstroidThrottlerGuard used but never imported; dropped an unused SlidingWindowThrottlerGuard import block - risk.service.ts: event-driven risk scoring built a RiskFactorsInput with fields (destination/velocityCount/isNewRecipient) that don't exist on the current type; mapped to the real shape instead - risk.service.spec.ts: removed orphaned unused fixtures - stellar.service.ts (src/modules/stellar/services, unused elsewhere in the app but still typechecked/tested): getTransactionInfo called a client method that doesn't exist (real method is getTransaction); simulateTransaction passed a bare string where the client expects an options object - stellar.service.spec.ts: rewritten against the real SorobanSimulationResult shape; fixed mockResolvedValueOnce/ mockRejectedValueOnce being consumed by the test's own first assertion, leaving the second call unmocked - transaction.service.spec.ts: rewritten against TransactionService's actual create() contract (it doesn't call Soroban simulation at all; the previous spec tested a flow that was never implemented) and a real Ed25519 checksum address - sensitive-rate-limit.integration.spec.ts: app.inject() doesn't exist on this Express-platform app; switched to app.listen + fetch, matching the sibling public-rate-limit.integration.spec.ts pattern, and named the test throttler 'api' so AstroidThrottlerGuard's tier-matching actually engages it
CI runs lint and test repo-wide, so these also blocked the PR: - 4 pre-existing no-explicit-any lint errors in throttler guard code and specs, typed properly instead of suppressed - 4 THROTTLE_* env vars (WEBHOOK_LIMIT, API_BURST, AUTH_BURST, WEBHOOK_BURST) were validated by the env schema but missing from docs/configuration.md, failing the docs-sync test
Every authenticated request performed a Redis round trip against the token blacklist to answer "is this session still revoked?". This adds a short-TTL caching layer in front of the blacklist so repeated verifications within one window skip the Redis query entirely. - Add CacheService: a small get/set/delete cache over the shared REDIS_CLIENT with TTL-bounded entries, JSON payloads, and SCAN-based prefix invalidation. Every operation degrades to a no-op/miss on Redis failure so caching can never break the request path. - Add TokenVerificationCacheService: caches per-session revocation answers for TOKEN_CACHE_TTL seconds (default 30, well below the 15-minute access-token lifetime) and exposes invalidation hooks. - Wire the cache into JwtStrategy.validate (cache-first, source of truth on miss, fail-open unchanged on Redis outages). - Hook invalidation into every revocation path: TokenBlacklistService drops the cached answer after each blacklist write (including on Redis-outage fallback), AuthService invalidates on logout and on refresh rotation, so revocations are observed immediately instead of after the TTL window. Revocation reliability is preserved because no cached answer outlives its TTL, and explicit logout/rotation clears the entry at once. Tests: unit suites for CacheService and TokenVerificationCacheService (hits, misses, resolver fallback, invalidation hooks), an integration suite proving repeated authentications trigger a single blacklist lookup and that logout flips a cached-valid session to 401 immediately, plus updated JwtStrategy/TokenBlacklistService/api-key integration suites for the new wiring. Closes ASTROIDX556#341 🤖 Generated with Codebuff Co-Authored-By: Codebuff <noreply@codebuff.com>
…d docs Get CI green for the token-verification cache PR by fixing pre-existing main-branch type errors alongside PR-specific ones: Express specs no longer use Fastify-only app.inject, Stellar mocks match the real Soroban result interface, the transaction spec exercises the actual create pipeline, TokenBlacklistService resolves the global REDIS_CLIENT token explicitly, and the configuration docs cover every THROTTLE_* env var the docs test asserts. 🤖 Generated with Codebuff Co-Authored-By: Codebuff <noreply@codebuff.com>
…ic routes Public endpoints are the first thing abusive traffic hits, but their limits were a single fixed IP budget: every public route shared PUBLIC_RATE_LIMIT_MAX_REQUESTS per window, and all clients behind one shared address (NAT, office egress, CI runners) exhausted one bucket together. This makes the public rate limiter configurable per route and per client identifier, on the existing Redis sliding-window counter. - Add @PublicRateLimit(max, windowSeconds) decorator: per-route (or per-controller) budget overrides resolved by the guard through Reflector; the global PUBLIC_RATE_LIMIT_* settings remain the default. - Add PUBLIC_RATE_LIMIT_CLIENT_IDENTIFIERS (optional, comma-separated, currently 'apiKey'): when enabled, a presented x-api-key or ApiKey/Bearer ak_... Authorization header is folded into the bucket key so distinct key-holding clients behind one IP get their own budgets. The IP always participates; keyless callers share the plain-IP bucket as before. - Extract a testable PublicRateLimitGuard.check() returning the full decision (allowed, limit, windowSeconds, count, resetAt); canActivate keeps its existing 429 + X-RateLimit-Limit/Remaining/Reset + Retry-After contract and in-memory fallback on Redis outage. Tests: unit suites for per-route rule resolution, identifier bucketing (with/without identifiers configured), header correctness on allowed and limited requests, and @SkipPublicRateLimit() interaction with rules; an HTTP-level integration suite simulating bursts that proves the 429-with-headers behaviour at the global limit, per-route overrides (next to unaffected sibling routes), and per-key budget isolation. Existing public-rate-limit suites pass unchanged (bucket keys keep the ip: prefix, so stored counters stay compatible). Closes ASTROIDX556#342 🤖 Generated with Codebuff Co-Authored-By: Codebuff <noreply@codebuff.com>
…d docs Get CI green for the token-verification cache PR by fixing pre-existing main-branch type errors alongside PR-specific ones: Express specs no longer use Fastify-only app.inject, Stellar mocks match the real Soroban result interface, the transaction spec exercises the actual create pipeline, TokenBlacklistService resolves the global REDIS_CLIENT token explicitly, and the configuration docs cover every THROTTLE_* env var the docs test asserts. 🤖 Generated with Codebuff Co-Authored-By: Codebuff <noreply@codebuff.com>
…able The duplicated helper also ignored its parameter while the config factory read the raw variable at the call site with none, so the module never compiled. Collapse to a single parser that takes the raw value. 🤖 Generated with Codebuff Co-Authored-By: Codebuff <noreply@codebuff.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
16 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements four major feature requests for enhanced API observability, security, and transaction simulation capabilities:
#247 - Prometheus Metrics Interceptor
#246 - Stellar Transaction Simulation Service
#245 - Cryptographic API Key Hashing Upgrade
#244 - Webhook Retry and Dead Letter Queue
Closes #247,
Closes #246,
Closes #245,
Closes #244