Skip to content

refactor(auth-mw): reduce cognitive complexity in auth-express-middleware index#85

Merged
sirdeggen merged 4 commits intomainfrom
sonar/w2-auth-mw
May 7, 2026
Merged

refactor(auth-mw): reduce cognitive complexity in auth-express-middleware index#85
sirdeggen merged 4 commits intomainfrom
sonar/w2-auth-mw

Conversation

@sirdeggen
Copy link
Copy Markdown
Contributor

Summary

Wave 2 cognitive complexity refactor — clear 5 S3776 violations in packages/middleware/auth-express-middleware/src/index.ts.

Approach

  • send() (complexity 18→2): Extracted sendGeneralMessage and sendNonGeneralMessage private methods; extracted readResponseHeaders helper
  • handleIncomingRequest() (complexity 32→5): Extracted handleWellKnownAuth, handleGeneralMessage, and handleUnauthenticated private methods
  • handleWellKnownAuth inner callbacks (complexity 23→split): Extracted registerCertificateListener and handleCertificatesForPeer private methods
  • General message listener (complexity 17→split): Extracted setupAuthenticatedResponse, hijackResponse, and scheduleNextOrCertificateWait private methods
  • writeBodyToWriter (complexity 28→7): Moved to authMiddlewareHelpers.ts with early-return pattern replacing nested if-else chain
  • buildAuthMessageFromRequest (complexity 17→5): Extracted writeUrlToWriter and writeRequestHeadersToWriter into helpers; replaced dual debug-log calls with makeDebugLogger closure
  • New helper file src/authMiddlewareHelpers.ts: Houses isLogLevelEnabled, getLogMethod, writeBodyToWriter, writeUrlToWriter, writeRequestHeadersToWriter, writeHeaderPair, convertValueToArray, makeDebugLogger; LOG_LEVELS and LOG_METHOD_MAP hoisted to module-level constants (efficiency); writeRequestHeadersToWriter deduped to use writeHeaderPair

Verification

  • pnpm --filter @bsv/auth-express-middleware run build
  • pnpm --filter @bsv/auth-express-middleware run test
  • 20 tests pass (no new tests needed — all existing integration tests cover the refactored paths)
  • Lint: pre-existing test-file warnings only (identical to main branch baseline); source files clean

Refs #38 #44

…ware index

Extract private methods (sendGeneralMessage, sendNonGeneralMessage, handleWellKnownAuth,
handleGeneralMessage, handleUnauthenticated, setupAuthenticatedResponse, hijackResponse,
scheduleNextOrCertificateWait, registerCertificateListener, handleCertificatesForPeer)
to break up five S3776 violations (complexity 17–32 → well below 15 each). Move
writeBodyToWriter, writeUrlToWriter, writeRequestHeadersToWriter, writeHeaderPair,
convertValueToArray, getLogMethod, isLogLevelEnabled into authMiddlewareHelpers.ts.
Hoist LOG_LEVELS and LOG_METHOD_MAP to module-level constants. Deduplicate header
serialization by routing writeRequestHeadersToWriter through writeHeaderPair.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Comment thread packages/middleware/auth-express-middleware/src/authMiddlewareHelpers.ts Dismissed
Comment thread packages/middleware/auth-express-middleware/src/index.ts Fixed
@codecov
Copy link
Copy Markdown

codecov Bot commented May 7, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

sirdeggen added 2 commits May 7, 2026 08:56
…atch alerts

- Validate header/query parameters are strings before use (CodeQL: js/type-confusion-through-parameter-tampering)
- Constrain log method dispatch to a closed set (CodeQL: js/unvalidated-dynamic-method-call)
Express returns header values as string|string[]|undefined. Direct
equality comparison silently fails for repeated headers (CodeQL:
js/type-confusion-through-parameter-tampering). Normalize via local
helper to preserve first-value semantics.
Comment thread packages/middleware/auth-express-middleware/src/authMiddlewareHelpers.ts Dismissed
Prior fix used a closure helper which CodeQL couldn't see through.
Inline narrowing makes the dataflow explicit at the comparison site,
clearing js/type-confusion-through-parameter-tampering alerts.
@sirdeggen
Copy link
Copy Markdown
Contributor Author

CodeQL note

Two critical CodeQL alerts (#103, #106) for js/type-confusion-through-parameter-tampering at authMiddlewareHelpers.ts:133,135 persist despite three escalating fixes:

  1. Attempt 1: Normalized header values via Array.isArray(v) ? v[0] : v ternary in writeRequestHeadersToWriter. Cleared two alerts; uncovered two more in writeBodyToWriter.
  2. Attempt 2: Added getHeader closure helper at top of writeBodyToWriter, extracted contentType once, replaced all headers['content-type'] comparisons.
  3. Attempt 3: Replaced closure with inline narrowing (if typeof === 'string' / Array.isArray) for visibility to CodeQL's dataflow analyzer. Strengthened body type guard with body !== null && typeof body === 'object' && !Array.isArray(body) before Object.keys(body) and URLSearchParams(body).

The remaining alerts appear to be false positives of the CodeQL query: contentType is provably a string at every comparison site (inline narrowing immediately above the if-block), and body is provably a non-array plain object at every consumption site. Tests (20/20) pass.

Suggest: triage the two alerts as false-positive at merge time, or merge as-is and have CodeQL re-evaluate against main (where the type-guard narrowing should be more obvious to the analyzer).

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented May 7, 2026

Quality Gate Failed Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

@sirdeggen sirdeggen merged commit 4946796 into main May 7, 2026
7 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants