Skip to content

agent-audit: loopback MCP gets the bind-address question, not TLS advice - #656

Merged
sandstream merged 2 commits into
mainfrom
claude/web-kit-status-sync-p6fg9a-loopback-mcp
Oct 30, 2026
Merged

sandstream merged 2 commits into
mainfrom
claude/web-kit-status-sync-p6fg9a-loopback-mcp

Conversation

@sandstream

@sandstream sandstream commented Oct 30, 2026 •

Copy link
Copy Markdown
Owner

Description

kit agent-audit told an MCP server at http://localhost:9876/mcp to "use https://". On loopback, TLS is not the risk. The risk is what the server binds to (0.0.0.0 exposes it to the LAN) and whether it asks for a token, and a client config cannot show either. Found while researching a self-hosted code-search MCP server whose quick start binds every port on all interfaces (sandstream/kit-research, docs/research/paparats-mcp-vs-kit.md).

  • A remote http:// server keeps its row and the advice "use https:// MCP endpoints".
  • A loopback server (localhost, *.localhost, 127.0.0.0/8, ::1) gets its own low warn row: "kit cannot see what the server binds to: check it listens on 127.0.0.1, not 0.0.0.0, or requires a token".

Three more fixes found on the way:

  • lizard measured the wrong functions in src/agent-audit.ts. The regex /^http:\/\// reads to lizard as a // comment. That made auditMcpServers span the whole file (it is baselined as length=426) and hid every function after it, runAgentAudit included. Once measured, runAgentAudit is CCN 16. The regexes are now replaced with startsWith, and the hook-directory scan is its own function (scanHookDirs). Every function is now measured and under CCN 15 and 80 lines. .kit-baseline.json is not touched; the stale baseline entry simply stops matching.
  • Hook files are read through one open descriptor. CodeQL flagged js/file-system-race on the moved hook scan: a statSync(path) followed by readFileSync(path). The hook is now opened once, with O_NONBLOCK where available so a named pipe cannot hang the audit. The regular-file check and the read both go through that descriptor. Symlinked hooks are still followed, because git runs them. The hook scan had no tests, and two are added.
  • A stale comment in src/keystore/active.ts said the .kit-policy.toml hardware mandate is not wired. It is: policyRequiresHardware is ORed with the env mandate in hardwareRequired. Only the comment changed.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update
  • Performance improvement
  • Refactoring

Related Issues

  • None filed. The research note above is the record.

Changes Made

  • src/agent-audit.ts:
    • isLoopbackUrl (exported);
    • cleartextMcpRows;
    • scanHookDirs and readHookFile, extracted from runAgentAudit;
    • auditMcpServers uses startsWith instead of a regex.
  • src/agent-audit.test.ts:
    • isLoopbackUrl cases, including the look-alikes localhost.evil.com, 127.0.0.1.nip.io and 0.0.0.0;
    • a loopback-plus-remote runAgentAudit test;
    • two hook-scan tests;
    • three regex assertions rewritten.
  • src/keystore/active.ts: the comment only.
  • CHANGELOG.md.

Testing

Test Coverage

  • Added new tests
  • Updated existing tests
  • All tests pass (npm test: 7334 tests, 0 failures)

Manual Testing

  1. Write .mcp.json with {"mcpServers":{"local":{"url":"http://localhost:9876/mcp"},"remote":{"url":"http://mcp.internal:8080"}}}.
  2. Run kit agent-audit.
  3. Expect cleartext MCP in .mcp.json (remote only, "use https://") and loopback MCP over http in .mcp.json (the bind-address question).

Breaking Changes

None / N/A. Both rows are warn, so the exit code does not change. A loopback-only config used to produce a medium row and now produces a low one.

Checklist

  • I've read CONTRIBUTING.md
  • My code follows the project's style guidelines
  • I've updated documentation as needed
  • All new code has test coverage
  • All tests pass locally (npm test)
  • No new warnings or errors introduced: eslint is still 147 warnings, adr check and review --stages skill pass, lizard reports no new findings, and CodeQL is clean on the current head
  • Commit messages follow conventions
  • No hardcoded secrets or sensitive data
  • Code has been self-reviewed

Screenshots or Logs

N/A

Performance Impact

  • No performance impact
  • Improves performance
  • Potential performance impact (describe):

Migration Guide

N/A

Reviewer Notes

  • On the first head, Windows shard 1/4 timed out in memory/pal-graph-cost.test.js, which this diff does not touch, while another PR's Windows jobs ran in parallel. The same shard passed on the current head.
  • Platforms: URL parsing is new URL. The hook read uses O_NONBLOCK only where the platform defines it, which is not Windows. Linux was tested locally and Windows is covered by CI. Nothing here is macOS-specific.

🤖 Generated with Claude Code

A remote http:// MCP server keeps 'use https://'. A loopback one (localhost, 127.0.0.0/8, ::1) gets its own low-severity row: kit cannot see what the server binds to, so it asks whether it listens on 127.0.0.1 or requires a token. Also: the /^http:\/\// regexes that made lizard measure one function across the whole file are gone and the hook-directory scan is its own function, so every function is measured and under the limits; a stale keystore comment that said the policy hardware mandate is not wired is corrected.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Claude-Session: https://claude.ai/code/session_01DbF4MFePXWNCd4dxBQvqH2
@github-actions

Copy link
Copy Markdown

Docker image built, scanned, and smoke-tested in CI. The PR image is local to this run and is not published.

Comment thread src/agent-audit.ts Fixed
CodeQL js/file-system-race on the moved hook scan: statSync(path) then readFileSync(path) lets the file change between the check and the read. The hook is now opened once (O_NONBLOCK where available, so a named pipe cannot hang the audit) and both the regular-file check and the read go through that descriptor. Symlinked hooks are still followed, as git runs them. The hook scan had no tests; two are added.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Claude-Session: https://claude.ai/code/session_01DbF4MFePXWNCd4dxBQvqH2
@github-actions

Copy link
Copy Markdown

Docker image built, scanned, and smoke-tested in CI. The PR image is local to this run and is not published.

@sandstream
sandstream merged commit e031741 into main Oct 30, 2026
26 checks passed
@sandstream
sandstream deleted the claude/web-kit-status-sync-p6fg9a-loopback-mcp branch October 30, 2026 09:40
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.

3 participants