Skip to content

fix(mcp): complete internal auth and audit loading - #13993

Merged
diegosouzapw merged 6 commits into
diegosouzapw:release/v3.8.51from
dakaribeckerer-art:fix/mcp-internal-auth-audit-runtime
Sep 29, 2026
Merged

diegosouzapw merged 6 commits into
diegosouzapw:release/v3.8.51from
dakaribeckerer-art:fix/mcp-internal-auth-audit-runtime

Conversation

@dakaribeckerer-art

Copy link
Copy Markdown
Contributor

Summary

  • Authenticate the MCP advanced telemetry, cache, and fastest-model helper fetches with the configured internal service token.
  • Allow the cache management route to accept that token only on verified loopback-local requests.
  • Load MCP audit's better-sqlite3 dependency through the existing standalone-safe runtimeRequire helper.
  • Add regressions for the helper fetches, cache trust boundary, and audit loader bundling.

Related Issues

Root Cause

The central MCP fetch path acquired internal-service authentication in #9260, but advancedTools.ts and pickFastestModel.ts use private apiFetch helpers. Those helpers continued forwarding only the caller's read-only bearer token to management routes, so valid read-only MCP calls failed with 403 Invalid management token. The cache route also did not recognize the internal service token.

Separately, the audit module dynamically imported node:module and called createRequire(import.meta.url). In the webpack standalone server bundle that loader can be rewritten to a non-callable module wrapper, causing a is not a function when audit writes run.

Security

  • External MCP API-key scopes are unchanged.
  • The internal service token remains separate from the caller bearer token.
  • /api/cache accepts the service token only with the server-stamped loopback locality proof.
  • The service-auth headers are applied last so caller-provided tool options cannot override them.

Validation

  • Change type: other (MCP / internal auth / build runtime)
  • Focused red-to-green regressions for both root causes
  • npm run test:vitest — 51 files, 474 tests passed
  • Targeted Node unit suite — 4 tests passed
  • npm run check:open-sse-typecheck
  • npm run typecheck:core
  • Targeted ESLint and pre-commit hooks
  • npm run build:contributor; inspected the emitted audit route and confirmed it calls runtimeRequire("better-sqlite3")
  • Reconciled with the current release/v3.8.51 head before validation
  • Production-code changes include automated regressions in this PR

Full npm run lint reached only the repository-wide stale-suppressions gate (There are suppressions left that do not occur anymore); linting the changed files passes. The full Node unit run progressed through the suite but was stopped after the pre-existing, source-documented Node runtime hang in proxyfetch-direct-response-start-timeout-10214.test.ts; no failure from a changed or related test was observed.

The same patch was also validated against a live Docker deployment: the affected read-only MCP tools succeeded, audit rows appeared through both storage and /api/mcp/audit, and logs no longer contained Invalid management token, the audit loader exception, or scope_denied for allowed read tools.

Tests Added Or Updated

  • open-sse/mcp-server/__tests__/audit.test.ts
  • open-sse/mcp-server/__tests__/httpAuthContext.test.ts
  • tests/unit/internal-service-auth.test.ts

Coverage Notes

The MCP Vitest suite covers the two private fetch helpers and the audit loader source contract. The Node unit test covers the cache route's loopback-only internal-service boundary. Production bundling validates the exact audit loading path that previously failed only after webpack transformation.

Reviewer Notes

No migration or external-key scope change is required. Operators still need OMNIROUTE_INTERNAL_SERVICE_TOKEN_FILE configured for the internal hop; missing configuration preserves the existing authentication behavior.

dakaribeckerer-art and others added 5 commits September 17, 2026 18:56
… test

Rebase-equivalent merge to clear the DIRTY/CONFLICTING state against the
current release tip. The only real conflict was a cosmetic comment/
indentation reflow in audit.test.ts's shutdown timeout comment (both
sides asserted identical behavior with the same 30000ms timeout); kept
this branch's version, which has the comment lines in original order.

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
# Conflicts:
#	open-sse/mcp-server/__tests__/audit.test.ts
#	open-sse/mcp-server/audit.ts
@diegosouzapw
diegosouzapw merged commit c8ab05f into diegosouzapw:release/v3.8.51 Sep 29, 2026
0 of 3 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