Commit Graph

3 Commits

Author SHA1 Message Date
Anso 4e5ba17710 refactor(backend): sanitize user input before logging to close CRLF injection (#807)
* refactor(backend): sanitize user input before logging to close CRLF injection

Adds a small sanitizeForLog helper that strips CR, LF, tab, and ASCII
control characters (0x00-0x1F, 0x7F) from a value before it is embedded
in a console.log/warn/error/debug call. Wraps every call site where a
user-controlled value (req.params, req.body, req.query, or a value
derived from them) flows into a log message.

Closes the bulk of the open CodeQL alerts in this family:
- 96 js/log-injection
- 28 js/tainted-format-string

The helper is in backend/src/utils/safeLog.ts. Routes still pre-validate
input at the request boundary; this is the second line of defense and
gives static analyzers a sanitizer they can trace through. JSON
responses, Docker filter labels, and other non-log call sites are
intentionally left unwrapped.

* refactor(backend): printf-style format strings for tainted-log call sites

CodeQL's js/tainted-format-string rule flags template literals in the first
arg of console.X when any interpolated value is user-controlled, regardless
of whether each value is sanitized inline. The canonical mitigation is to
use a static format string and pass values as positional args.

Converts the 28 flagged template literals to printf-style ("%s") format
strings, with sanitizeForLog applied to each positional arg. Also fills in
the log-injection wraps on 9 sites where a user-controlled value was
missed in the first sweep (agents, fleet, gitSources, imageUpdates,
GitSourceService).

No behavior change at runtime. Node's util.format substitutes %s tokens
identically to template-literal interpolation.

* fix(backend): wrap nodeId/snapshotId in fleet restore debug log

CodeQL flagged the unwrapped numeric args even though they cannot
contain control chars in practice. Apply the sanitizer for taint-flow
recognition.
2026-04-27 10:47:23 -04:00
Anso ca5a930c68 refactor(backend): extract authMiddleware and introduce createApp factory (phase 2) (#732)
Phase 2 of the index.ts refactor. Pulls the auth middleware and session
cookie issuers into their own module, and introduces the app.ts factory
that owns the first nine steps of the canonical middleware pipeline.

New modules:
- middleware/auth.ts: authMiddleware, issueSessionCookie,
  issueMfaPendingCookie, clearMfaPendingCookie
- app.ts: createApp() factory installing trust proxy, helmet, cors,
  compression, cookieParser, rate limiters, conditionalJsonParser, and
  nodeContextMiddleware. A header comment documents all 16 canonical
  middleware steps and where each currently lives.

Changes:
- middleware/authGate.ts: createAuthGate factory removed; authGate now
  imports authMiddleware directly (the factory existed only to avoid a
  circular import while authMiddleware lived in index.ts).
- services/DatabaseService.ts: added API_TOKEN_SCOPE_TO_ROLE map so the
  auth middleware no longer inlines a stringly-typed record.
- index.ts drops ~260 lines; auth routes, authGate, auditLog,
  apiTokenScope, remaining routes, static serving, and the error handler
  continue to be registered there until their respective phases.
- vitest.config.ts bumps testTimeout to 30s and hookTimeout to 45s so
  fork-pool workers have enough headroom to ts-node-transform the
  growing module graph under CPU contention (64 workers each import the
  full Express stack in beforeAll).

Code review fixes: use getErrorMessage() util in the auth catch block
instead of inline cast; promote the scope-to-role map to a typed
module-level constant.
2026-04-23 19:02:13 -04:00
Anso 856a260a11 refactor(backend): extract rate limiters, body parsing, and request gates (phase 1) (#731)
Phase 1 of the index.ts monolith refactor. Extracts all non-auth middleware
into focused modules. Routes and authMiddleware itself stay in index.ts for
now; Phase 2 introduces createApp() and extracts authMiddleware.

New modules:
- middleware/rateLimiters.ts: globalApiLimiter, pollingLimiter,
  webhookTriggerLimiter, authRateLimiter, ssoRateLimiter, trivyInstallLimiter
  plus the hybrid rateLimitKeyGenerator and isNodeProxyRequest helper
- middleware/jsonParser.ts: conditionalJsonParser that preserves the raw
  stream for remote-proxy forwarding (via helpers/proxyExemptPaths)
- middleware/nodeContext.ts: nodeContextMiddleware
- middleware/apiTokenScope.ts: enforceApiTokenScope + DEPLOY_ALLOWED_PATTERNS
- middleware/authGate.ts: createAuthGate(authMiddleware) factory + auditLog.
  Factory takes authMiddleware as a dependency to avoid a circular import
  until Phase 2 extracts the auth module.
- middleware/errorHandler.ts: central error handler that preserves
  err.status / err.expose from body-parser and other HTTP errors
- helpers/routePatterns.ts: WEBHOOK_TRIGGER_RE shared by rateLimiters and
  authGate

index.ts drops ~290 lines. Middleware registration order is unchanged.
All 1278 tests pass.

Code review fixes: typed ApiTokenScope in apiTokenScope.ts; replaced 2
em dashes with colons (Directive 18); added local CachedProxyFlagReq
type alias for the node_proxy memoization cast; extracted deny() helper.
2026-04-23 18:22:26 -04:00