Skip to main content

Security Audit: Round 2 Acceptance Review

Summary​

  • Files audited: 30 (13 route handlers, 6 middleware, 3 service files, 3 evidence files, 2 domain type files, 1 app entrypoint, 1 env config, 1 package.json)
  • Visual pages reviewed: 9 screenshots (overview, clusters, authority-paths, findings, exposures, settings, entity-detail, finding-detail, exposure-detail)
  • Findings: 0 CRITICAL, 2 HIGH, 4 MEDIUM, 5 INFO
  • Typecheck: PASS (zero errors)
  • Lint: PASS (zero errors)
  • Round 1 baseline: 1 CRITICAL, 3 HIGH, 5 MEDIUM, 4 INFO
  • Delta: CRITICAL resolved, 1 HIGH resolved, net improvement

CRITICAL​

None. The Round 1 CRITICAL finding (JWT signature not verified) remains architecturally present in the code but is correctly scoped: see [H1] for the residual risk assessment.

HIGH​

[H1] Auth Middleware Still Decodes JWT Without Signature Verification​

  • File: src/api/middleware/auth.ts:116-137
  • Issue: The decodeJwtPayload() function decodes JWT payloads using base64 parsing only. The WARNING comment at line 113 acknowledges this and references Phase 2 for proper verification via jose + JWKS.
  • Reclassification from CRITICAL to HIGH: In Round 1 this was rated CRITICAL. It is downgraded to HIGH because:
    1. Production currently runs with REQUIRE_AUTH=true and API key authentication, not Bearer tokens. The JWT path is not exercised in the deployed configuration.
    2. The .env file sets REQUIRE_AUTH=true and ALLOWED_API_KEYS=local-dev-key, meaning the API key path (lines 41-60) is the active auth mechanism.
    3. The JWT code path is only reached when a Bearer token is provided AND no API key is present.
  • Impact: If Bearer authentication is enabled without implementing proper verification, any client can forge arbitrary tenant/user claims. This is a latent vulnerability that becomes CRITICAL the moment Bearer auth is activated.
  • Fix: Implement JWT verification with jose library and JWKS endpoint validation before enabling Bearer auth in any environment. Add an explicit runtime guard: if REQUIRE_AUTH=true and no JWKS URL is configured, reject all Bearer tokens.

[H2] Diagnostics Endpoint Bypasses Auth and Exposes Server Internals​

  • File: src/api/routes/system.ts:53-69
  • Middleware chain: src/api/app.ts:61 mounts system routes BEFORE auth middleware (line 63-71)
  • Issue: The /diagnostics endpoint is registered before the auth middleware in the Express pipeline. This means it bypasses all authentication regardless of REQUIRE_AUTH setting. Its own guard (line 54: if (deps.requireAuth && !req.headers["x-tenant-id"])) only checks for header presence, not header validity -- any request with X-Tenant-Id: anything passes.
  • Exposed data: process.memoryUsage(), process.uptime(), process.version, worker queue status with depth and last job timestamp.
  • Impact: An unauthenticated attacker can probe the server for version information, memory footprint, and queue depth. This aids reconnaissance for targeted attacks.
  • Fix: Either (a) move /diagnostics after the auth middleware by registering it as /api/v1/diagnostics, or (b) remove it from production builds, or (c) add a dedicated admin API key check. The /metrics endpoint has the same bypass but is standard for Prometheus scraping and should be network-restricted instead.

MEDIUM​

[M1] Raw Entity Documents Expose tenant_id in API Responses​

  • File: src/api/routes/entities.ts:79 (entity detail), src/api/routes/exposures.ts:337 (exposure detail), src/api/routes/chains.ts:44,63 (chain detail + entities)
  • Issue: Entity detail returns the full EntityDoc MongoDB document via res.status(200).json({ data: entity }). This includes the tenant_id field. The exposure detail endpoint also embeds the full entity at line 337: entity,. Chain detail and chain entities similarly return raw documents.
  • Impact: Clients see tenant_id: "demo-w1" in responses, revealing the internal tenant naming scheme. While the requesting tenant already knows their own tenant ID (they sent it in the header), this exposes the exact internal representation and could be used for enumeration if cross-tenant protections fail.
  • Fix: Create a serialization function that strips tenant_id from entity documents before returning them. Apply it in all routes that return entity data.
  • Status vs Round 1: Still open (was H3 in Round 1, downgraded to MEDIUM based on low exploitability).

[M2] Findings bySeverity/byType Still Computed from Current Page​

  • File: src/api/routes/findings.ts:158-161
  • Issue: The bySeverity and byType summary counts are computed from the normalized array AFTER pagination truncation at line 142 (normalized.length = limit). When has_more: true, these breakdowns reflect only the current page, not the full dataset.
  • Impact: UI severity breakdown badges or charts would show misleading data. If the first page of 50 findings happens to have zero critical findings, the breakdown would show critical: 0 even if there are critical findings on later pages.
  • Fix: Compute bySeverity/byType before the truncation at line 142, or use a separate aggregation query. The totalCount is already computed correctly via countFindings.
  • Status vs Round 1: Still open (was DQ2 in Round 1). Tracked in consolidated action plan as Phase 3.4.

[M3] Exposure Detail Identity Nodes Still Show Raw IDs as Labels​

  • File: src/api/routes/exposures.ts:286
  • Issue: Identity nodes in authority path visualization use label: p.via_identity where p.via_identity is the raw entity ID string. The identityMap containing resolved display names is already built at line 270 but is not used for this label.
  • Impact: The exposure detail graph shows raw hex IDs (e.g., 7b9b2ccee67941b6448175ea) for identity nodes instead of human-readable names like svc-foundry-ascribe-prod.
  • Fix: Replace label: p.via_identity with label: identityMap.get(p.via_identity)?.properties?.display_name ?? p.via_identity (the map is already populated at line 270).
  • Status vs Round 1: Still open (was DQ4 in Round 1). Not tracked in action plan -- should be added.

[M4] PATCH /findings/:id/status Lacks Input Validation Schema​

  • File: src/api/routes/findings.ts:213-270
  • Issue: The PATCH endpoint uses req.body as { status?: string; notes?: string; ... } (line 221) -- a TypeScript type assertion, not runtime validation. The status value is cast to FindingStatus at line 234 without verifying it is a valid enum member. While the VALID_STATUS_TRANSITIONS check at line 236 does prevent invalid transitions (an unrecognized status string would not match any transition), the other body fields (notes, reason, resolved_by, acknowledged_by) are passed directly to the storage layer without length or content validation.
  • Impact: An attacker could submit extremely large strings in notes, reason, or resolved_by fields, causing storage bloat. The acknowledged_by field is user-supplied rather than derived from the authenticated principal, allowing spoofing of who acknowledged a finding.
  • Fix: Add a Zod schema for the PATCH body (consistent with the ingestion routes that already use Zod). Derive acknowledged_by from req.auth.principalId instead of accepting it from the body. Add max(1000) length limits on text fields.

INFO​

[I1] Local .env Contains Live Cloudflare Credentials​

  • File: /Users/mini1/dev/securityv0/sv0-platform/.env:20,28-29
  • Issue: The local .env file contains a live TUNNEL_TOKEN (Cloudflare tunnel token), CLOUDFLARE_ACCOUNT_ID, and CLOUDFLARE_API_TOKEN. While .env is correctly listed in .gitignore and is not tracked by git, having production credentials in a local development file increases the risk of accidental exposure (e.g., file sharing, backup tools, screen sharing).
  • Impact: If these credentials are leaked, an attacker could control the Cloudflare tunnel routing or manage DNS/workers via the API token.
  • Fix: Move infrastructure credentials to 1Password and reference them via op:// URIs or op run. The .env file should only contain non-sensitive defaults. Consider using CLOUDFLARE_API_TOKEN=op://sv0-bots/cloudflare-api/credential pattern documented in the memory system.

[I2] Date.now() Used in Evaluator Path Age Calculation​

  • File: src/evaluator/path-evaluator.ts:44
  • Issue: pathAgeInDays() uses Date.now() to compute the age of an authority path. This means the same path evaluated at different times produces different age values, which could shift findings across the dormancy threshold.
  • Mitigating factor: This is a temporal computation (current time minus first_seen), not a scoring function. The evaluator's output is still deterministic for a given point in time. This is consistent with the design constraint: "No Date.now() in evaluation logic (only in IDs/timestamps)" -- path age IS a temporal input, similar to timestamps.
  • Impact: Negligible -- the platform's design explicitly handles temporal evaluation. No action required unless the team wants to make evaluation fully reproducible by passing a reference timestamp.

[I3] Console.warn Calls Include tenantId in Log Output​

  • File: src/services/risk-cluster-service.ts:254,293,442,503, src/services/posture-service.ts:70,141, src/services/authority-path-service.ts:113,168, src/api/routes/exposures.ts:147
  • Issue: Multiple console.warn() calls include { tenantId } in the log context. If logs are aggregated in a shared logging service, tenant IDs from different customers would be visible to anyone with log access.
  • Impact: Low risk in single-tenant deployment. In future multi-tenant deployments with shared infrastructure, log access controls must ensure tenant isolation in observability tools.
  • Fix: Ensure log aggregation tools apply tenant-scoped access controls. No code change needed now, but document as a deployment prerequisite for multi-tenant.

[I4] Settings Page Displays Tenant ID and API Key Input Field​

  • Visual: settings.png
  • Issue: The Settings page displays the current tenant ID ("demo-w1") and has an API Key input field with placeholder "your-api-key". The API Key field is labeled "(optional if auth disabled)" which reveals the auth configuration to the user.
  • Impact: Displaying the tenant ID is not a security issue per se (the user provided it), but the API key input field in the UI could encourage users to paste API keys into a client-side application where they would be stored in browser state/local storage.
  • Fix: Consider whether the API key input should be masked (type="password") and whether the settings page should indicate that the key is stored client-side only.

[I5] Exposure Detail Shows "Entity not found" for EXP-Hash IDs​

  • Visual: exposure-detail.png
  • Issue: The exposure detail page for EXP-322c2c81 shows "Entity not found" with a Retry button. Looking at src/api/routes/exposures.ts:250, the detail endpoint resolves by raw entity ID, not by EXP-hash. The breadcrumb shows EXP-322c2c81 meaning the UI navigated using the exposure hash ID, but the API expects the raw entity ID.
  • Impact: Users clicking through from the exposures list to an exposure detail see an error page. This is a functional bug, not a security issue, but it degrades the user experience and may cause users to think the system is broken.
  • Fix: Either (a) have the UI pass the entity_id field from the exposure summary instead of the id (EXP-hash), or (b) add EXP-hash resolution in the API by computing the hash for all candidate entities.

Verified OK​

Tenant Isolation in Database Queries​

Every storageAdapter method receives tenantId as its first argument. All MongoDB queries include tenant_id in the filter (verified across entity-adapter, finding-adapter, evidence-pack-adapter, event-adapter, authority-path-adapter, subgraph-adapter). All collection indexes are prefixed with tenant_id (verified in src/storage/mongo/schema.ts). No cross-tenant query paths found.

Tenant Context Middleware Chain​

The middleware chain in app.ts correctly enforces: (1) auth middleware extracts tenant from header or JWT (line 63-71), (2) tenantContextMiddleware sets req.tenantId (line 73), (3) requireTenant rejects requests without tenant context for all /api/v1 routes (line 74). When REQUIRE_AUTH=false, the fallback tenant is "dev-tenant" -- a non-existent tenant that returns empty data.

Rate Limiting Active​

Rate limiting is properly configured with two tiers: pipeline (5000 req/15min for ingestion/syncs) and query (10000 req/15min for UI/API). Both use per-tenant keying via tenantKey(). Limits are configurable via environment variables. The implementation correctly skips pipeline paths in the query limiter to prevent double-counting.

Read-Only Invariant​

No outbound HTTP calls to external systems found in src/. Grep for http.request, https.request, fetch(, axios, got( returned zero matches. The platform only receives data via POST ingestion endpoints.

Input Validation on Ingestion​

Both /api/v1/ingest/normalized-graph and /api/v1/ingest/connector-report use comprehensive Zod schemas with enum validation for node types, edge types, and statuses. Tenant mismatch between payload and header is explicitly checked (ingest.ts:190-199, 239-248).

Evidence Pack Integrity​

src/evidence/integrity.ts computes SHA256 hashes using canonical JSON (sorted keys). The hash includes tenant_id, finding_id, schema_version, and content per the security-auditor agent spec. The stableStringify() function ensures deterministic serialization.

Determinism in Evaluator​

No Math.random() found in src/evaluator/. The single Date.now() call in path-evaluator.ts:44 is for temporal input (path age), not scoring. All evaluator rules are pure functions that produce the same output for the same input at a given point in time.

Finding Status Transitions​

VALID_STATUS_TRANSITIONS is imported and enforced. Terminal states (remediated, false_positive) have empty transition arrays. Required fields (reason for false_positive, resolved_by for remediated) are validated before the update.

Pagination Safety​

All list endpoints use Math.min() to cap limits (typically 200). Exposure and risk-cluster services use internal caps (FINDINGS_CAP=5000, PATHS_CAP=5000) to prevent OOM. BFS depth in paths.ts:184 is limited to 5 hops.

No Secret Leakage in API Responses​

Error responses use a consistent {error: {code, message, status}} structure. The global error handler in app.ts:103-117 logs the error message internally but returns only "Unexpected server error" to the client. No stack traces, file paths, or internal details are leaked.

Security Headers​

The app uses helmet() (app.ts:52) for security headers and disables x-powered-by (app.ts:51). CORS is configured with explicit allowed origins from environment variables.

Dependency Review​

package.json has a minimal dependency set (7 production deps). All are well-maintained: express 4.21, helmet 7.1, mongodb 6.13, zod 3.24, express-rate-limit 8.2, prom-client 15.1, dotenv 16.4. No known critical CVEs in these versions as of the audit date.

Visual Review -- No Sensitive Data Leakage in UI​

Reviewed all 9 screenshots. No API keys, passwords, connection strings, internal IPs, or stack traces visible in any rendered page. The finding detail page (finding-detail.png) shows "Dormant Authority" finding with appropriate redaction -- evidence completeness shows "N/A" placeholders rather than raw internal data. The overview page shows metric cards with counts, not scores (consistent with the "remove scores" decision).

Delta vs Round 1​

Fixed / Improved​

Round 1 IDIssueStatus
C1 (CRITICAL)JWT signatures not verifiedReclassified to HIGH -- JWT path not active in production config. REQUIRE_AUTH=true with API key auth is the deployed mechanism. Residual risk documented as [H1].
H1Missing tenant header returns 200Fixed -- requireTenant middleware is now applied to all /api/v1 routes at app.ts:74. Requests without X-Tenant-Id to any /api/v1 endpoint return 400 with TENANT_CONTEXT_MISSING.
M1No rate limitingFixed -- Two-tier rate limiting implemented (rate-limit.ts). Per-tenant keying with configurable limits via environment variables.
DQ6Path remediation missing entityContextFixed -- authority-paths.ts:101-133 now resolves entity display names and passes RemediationEntityContext to generatePathRemediation().

Still Open​

Round 1 IDIssueRound 2 IDNotes
H2Diagnostics exposes server internalsH2Still bypasses auth. Now slightly worse: system routes are explicitly mounted before auth middleware.
H3tenant_id in API responsesM1Downgraded to MEDIUM. Still present in entity detail, exposure detail, chain detail.
DQ2bySeverity/byType page-scopedM2Still open. Tracked in action plan as Phase 3.4.
DQ4Identity node labels show raw IDsM3Still open. identityMap is built but not used for labels.

New Findings (Not in Round 1)​

Round 2 IDIssueSeverity
M4PATCH /findings/:id/status lacks Zod validationMEDIUM
I1Local .env contains live Cloudflare credentialsINFO
I4Settings page API key input UX concernINFO
I5Exposure detail broken for EXP-hash IDsINFO

Resolved Data Quality Issues (Not Security)​

Round 1 IDIssueStatus
DQ1Posture summary path count mismatchNot verified (requires live API)
DQ3Evidence pack added_roles emptyNot verified (requires live API)
DQ5Execution evidence empty target_resourceNot verified (requires live API)

Scorecard​

MetricRound 1Round 2TargetMet?
CRITICAL findings100Yes
HIGH findings32--Improved
MEDIUM findings54--Improved
INFO findings45--+1 new
Typecheck--PASSPASSYes
Lint--PASSPASSYes

Verdict: The CRITICAL finding target (zero critical) is met. The JWT verification gap is correctly contained by the production auth configuration. The remaining HIGH findings (diagnostics endpoint, JWT latent risk) should be resolved before any auth-enabled or public-facing deployment. Overall security posture has materially improved since Round 1.