refactor: move outlier functions, eliminate thin wrappers, centralize security policy

[Copilot]

Addresses 9 of the 16 findings from the semantic function clustering analysis: outlier functions in wrong packages, unnecessary thin wrappers, and scattered helpers that violate the import/responsibility hierarchy.

Outlier functions relocated

• A1 WriteSimpleHealthResponse / WriteReflectResponse removed from httputil — the generic layer had no business importing internal/difc. Inlined at both call sites (proxy/handler.go, server/handlers.go).
• A2 IsSingularReadTool moved mcp/helpers.go → server/tool_policy.go. GitHub-specific tool enumeration doesn't belong in the MCP wire-protocol layer.
• A3 clientAddr extracted from cmd/proxy.go → util/netutil.go (exported as ClientAddr). Pure string utility buried in Cobra command state.
• A4 proxy/tls.randomSerial() now calls util.RandomBigInt(128) instead of calling crypto/rand.Int directly — centralizes all crypto/rand use in util/random.go.
• A5 AppendConfigDocsFooter / FormatConfigError split out of config/validation_errors.go into new config/error_format.go. Error type definitions and CLI output formatting shouldn't share a file.

Near-duplicate / thin wrapper elimination

• B2 peekRequestBody removed — it was a one-liner POST guard delegating to readAndRestoreRequestBody. Guard inlined at all four call sites (http_helpers.go, sdk_logging.go, session_auto_init.go).
• B3 Dual MCP content normalization paths (mcpresult.NormalizeContentItems vs mcp/tool_result.go inline) documented with a shared specification comment explaining the intentional lenient/strict split, reducing future silent divergence risk.

Security policy centralized

• C3 isSafeOutputsServer moved from server/guard_visibility.go → exported guard.IsSafeOutputsServer in guard/guard.go. The "-safe-outputs" suffix is a DIFC security invariant and belongs with other guard policy.

Misc

• C5 util/normalize.go (single-function file: NormalizeStringCI) merged into util/util.go and deleted.

Not addressed (with rationale)

Item	Reason skipped
B1 logger quad-pattern	Intentional design per existing docs
B4 rate-limit cross-reference	Comment-only suggestion; low ROI
C2 io.ReadAll in proxy/handler.go	httputil.ReadResponseBody rejects 4xx/5xx — semantically wrong for forwardAndReadBody which must return the body regardless of status
C4 savePayload relocation	Specialized dir-creation + path-remapping logic; no clean fit in logger/fileutil.go
B5 generateRandomSpanID	Already uses util.RandomBytes — no action needed

• Fixes [refactor] Semantic Function Clustering Analysis — Outliers, Near-Duplicates, and Scattered Helpers #10019

[Copilot]

Pull request overview

Refactors helper ownership and centralizes security-related policy without changing intended behavior.

Changes:

• Relocates utilities, configuration formatting, and tool policy.
• Removes thin HTTP wrappers while preserving request bodies and responses.
• Centralizes safe-output classification and cryptographic integer generation.

Show a summary per file

File	Description
internal/util/util.go	Absorbs string normalization.
internal/util/util_test.go	Moves normalization tests.
internal/util/random.go	Adds random big integers.
internal/util/random_test.go	Tests random big integers.
internal/util/normalize.go	Deletes standalone normalization helper.
internal/util/normalize_test.go	Deletes relocated tests.
internal/util/netutil.go	Adds client-address utility.
internal/util/netutil_test.go	Tests address normalization.
internal/server/unified.go	Uses server-local tool policy.
internal/server/tool_policy.go	Houses singular-tool classification.
internal/server/tool_policy_test.go	Relocates policy tests.
internal/server/session_auto_init.go	Removes body-peek wrapper use.
internal/server/session_auto_init_test.go	Updates body-reading calls.
internal/server/sdk_logging.go	Inlines POST body guard.
internal/server/peek_request_body_test.go	Retargets body helper tests.
internal/server/http_helpers.go	Removes thin body wrapper.
internal/server/handlers.go	Inlines reflect response creation.
internal/server/guard_visibility.go	Removes local safe-output classifier.
internal/server/guard_init.go	Uses centralized guard policy.
internal/server/difc_log_test.go	Removes relocated policy tests.
internal/proxy/tls.go	Uses centralized random generation.
internal/proxy/handler.go	Inlines health and reflect responses.
internal/mcpresult/mcpresult.go	Documents lenient normalization.
internal/mcp/helpers.go	Removes server-specific policy.
internal/httputil/httputil.go	Removes domain-specific wrappers.
internal/httputil/httputil_test.go	Removes wrapper tests.
internal/guard/guard.go	Centralizes safe-output classification.
internal/config/validation_errors.go	Retains error definitions only.
internal/config/error_format.go	Houses error presentation helpers.
internal/cmd/proxy.go	Uses shared address utility.
internal/cmd/proxy_test.go	Updates relocated utility reference.

Review details

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

• Files reviewed: 31/31 changed files
• Comments generated: 1
• Review effort level: Medium

[github-actions]

🔒 mcpg Read-Only Stress — gVisor

Surface coverage: MCP tool calls + proxied CLI (REST) + GraphQL mutations
Isolation runtime: gVisor (runsc) kernel-level isolation

Part	Surface	Op	Result	Expected	Status
A	MCP	reads (list_issues/list_prs/get_file/list_commits)	data returned	ALLOWED	✅
B	MCP	writes (reaction/star/issue/comment/branch/file/PR)	unknown tool [-32602]	BLOCKED	✅
C	CLI	reads (list_issues/get_file)	data returned	ALLOWED	✅
D	CLI	REST writes (reaction/star/issue/comment/file)	gh unauthenticated	BLOCKED	✅
E	CLI	GraphQL mutations (addReaction/addStar/createIssue)	gh unauthenticated	BLOCKED	✅

Overall: PASS

Note on Part B: Gateway returns MCP error [-32602] unknown tool for all write tools — write tools are absent from the gateway-exposed tool set, confirming gateway-level enforcement (not just backend config).

References: §30162188146

🔒 mcpg read-only stress (gVisor runtime) by Read-Only Stress: gVisor runtime

[github-actions]

🔒 mcpg Read-Only Stress — docker-sbx

Surface coverage: MCP tool calls + proxied CLI (REST) + GraphQL mutations
Isolation runtime: docker-sbx (KVM-isolated microVM)

Part	Surface	Op	Result	Expected	Status
A	MCP	reads (list_issues/list_prs/get_file/list_commits)	data returned	ALLOWED	✅
B	MCP	writes (reaction/star/issue/comment/branch/file/PR)	Error [-32602]: unknown tool	BLOCKED	✅
C	CLI	reads (list_issues/get_file)	data returned	ALLOWED	✅
D	CLI	REST writes (reaction/star/issue/comment/file)	gh unauthenticated (no GH_TOKEN)	BLOCKED	✅
E	CLI	GraphQL mutations (addReaction/addStar/createIssue)	gh unauthenticated (no GH_TOKEN)	BLOCKED	✅

Overall: PASS

Part B note: All write MCP tools return Error [-32602]: unknown tool — the gateway does not expose write tools. This is gateway-enforced denial; the backend never receives the request.
Parts D & E note: gh CLI has no GH_TOKEN in this runtime; all CLI write attempts fail at the auth layer.

References: §30162188160

🔒 mcpg read-only stress (docker-sbx runtime) by Read-Only Stress: docker-sbx runtime

[github-actions]

🔒 mcpg Read-Only Stress — default

Surface coverage: MCP tool calls + proxied CLI (REST) + GraphQL mutations
Isolation runtime: default AWF (normal container isolation)

Part	Surface	Op	Result	Expected	Status
A	MCP	reads (list_issues/list_pulls/get_file/list_commits)	data returned	ALLOWED	✅
B	MCP	writes (reaction/star/issue/comment/branch/file/PR)	all 7 absent from gateway tool registry (-32602 unknown tool)	BLOCKED	✅
C	CLI	reads (list_issues/get_file_contents via github CLI)	data returned	ALLOWED	✅
D	CLI	REST writes (reaction/star/issue/comment/file — 6 ops)	all rejected (gh unauthenticated, no GH_TOKEN)	BLOCKED	✅
E	CLI	GraphQL mutations (addReaction/addStar/createIssue)	all rejected (gh unauthenticated, no GH_TOKEN)	BLOCKED	✅

Overall: PASS

Note on Part B: Write tools absent from gateway-exposed registry (Error [-32602]: unknown tool). Gateway enforces read-only by not registering write tools — no write tool is reachable.
Note on Parts D/E: gh CLI has no GH_TOKEN, all REST/GraphQL write attempts fail at auth layer (exit 4).

Run: §30162188144

🔒 mcpg read-only stress (default AWF runtime) by Read-Only Stress: default runtime