feat(consent): add interactive consent and CI mode (Phase 3)

[appleboy]

Summary

Phase 3 of the agent-scan → Go port: adds interactive per-server consent before launching stdio MCP servers as subprocesses, and a CI mode that turns findings/runtime failures into a non-zero exit. This is a fully local feature with no backend dependency.

AI Authorship

• No AI was used in this PR
• AI was used. Details:
  • Tool / model: Claude Code (Opus 4.7), executing Phase 3 of plan.md
  • AI-authored files: all files in this PR
  • Human line-by-line reviewed: ⚠️ none yet — relying on the test suite + reviewer. Treat every changed file as needing close attention.

Change classification

• Core code — touches internal/pipeline/, internal/inspect/, and internal/models/ (the inspection path used by both scan and inspect). A bug here affects every scan.

Plan reference

Phase 3 of plan.md ("Interactive consent + CI mode"). Scope implemented:

• New internal/consent/ — CollectConsent(clients, out, in) mirroring the Python consent.py: enumerate stdio/remote servers, render shell-quoted command + redacted env, prompt [y/N] per stdio server (empty/EOF = decline), auto-allow remote, return declined set.
• --dangerously-run-mcp-servers (skip prompts, run all), --ci, --ignore-issues-codes.
• --ci requires --dangerously-run-mcp-servers; --ignore-issues-codes only valid with --ci (both → exit code 2).
• Pipeline gates stdio startup via a ConsentFn hook; inspector skips declined stdio servers (remote always inspected).
• CI exit: apply ignore codes, then exit 1 if any issue or runtime-failure (X-code) remains.

Context — why Phases 1 & 2 were skipped: the user confirmed this fork's backend has no client-bootstrap, guard-enabled, or push-key endpoints (plan.md Open Question #1), so Bootstrap (Phase 1) and Guard (Phase 2) would be permanently inert. Phase 3 and the upcoming Phase 4 are backend-independent and proceed normally.

Interactive detection mirrors agent-scan: inspect is always interactive; scan is interactive unless a control server carries an x-client-id header (automated run → prompts auto-skipped).

Verification

• Unit tests — consent rendering / shell-quoting / env redaction / allow+decline / EOF-declines-all; CLI flag validation; push-key detection; buildConsentFn branches; CI exit logic (ignored W001 passes, E-code & runtime-failure fail); inspector skip-declined behavior
• More than 3 test cases (happy path + multiple error/edge cases)
• Stress / soak: N/A (no long-running/async additions)
• Local gates: make fmt, make lint (0 issues), make test (race + coverage) all pass.
• Manual e2e (all plan scenarios) confirmed:
  • scan <cfg> --dangerously-run-mcp-servers → no prompt, prints the "without prompting" banner
  • scan <cfg> + piped n → prompts, env shown as KEY=*** (secret never echoed), server declined/skipped
  • scan --ci (no --dangerously) → exit 2 with a clear message
  • scan --ignore-issues-codes W001 (no --ci) → exit 2

Security check

• No secrets in code; env values are deliberately never echoed (rendered as KEY=***)
• External inputs (stdin answers) validated — only y/yes allow; everything else (incl. EOF) declines
• No new network surface (purely local prompting + exit-code logic)

Risk & rollback

• Risk: Behavior change — scan/inspect now prompt before starting stdio servers in interactive runs. Automated/piped runs with no TTY input will decline all stdio servers unless --dangerously-run-mcp-servers (or an x-client-id control-server header) is set. CI pipelines must add --dangerously-run-mcp-servers.
• Rollback: Self-contained additive feature on a branch — revert the commit to restore non-prompting behavior.

Reviewer guide

• Read carefully:
  • internal/consent/consent.go — prompt flow, env redaction (no value leakage), shell quoting, EOF=decline
  • internal/cli/consent.go — flag validation (--ci⇒dangerous; --ignore-issues-codes⇒--ci), interactive/push-key detection, CI exit semantics, the os.Exit paths
  • internal/inspect/inspector.go — the declined-stdio skip (and that remote is unaffected)
  • internal/pipeline/pipeline.go — ConsentFn hook placement (runs before any subprocess starts)
• Spot-check OK:
  • internal/cli/{scan,inspect,flags}.go wiring, internal/models/client.go (ServerRef + Declined field), README Options update, the test files

🤖 Generated with Claude Code

[Copilot]

Pull request overview

This PR adds interactive, per-stdio-server consent gating before launching MCP stdio servers as subprocesses, plus a CI mode that converts remaining findings/runtime failures into a non-zero exit code. This extends the scan/inspect pipeline with a consent hook and introduces CLI validation/exit semantics intended for automated environments.

Changes:

• Add internal/consent prompting + redacted command/env rendering, and wire consent decisions into the pipeline/inspector.
• Add CLI flags --dangerously-run-mcp-servers, --ci, and --ignore-issues-codes, including validation and CI exit behavior.
• Update docs and add unit tests for consent prompting, declined-server skipping, flag validation, and CI exit logic.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
README.md	Documents new consent/CI flags and interactive behavior.
internal/pipeline/pipeline.go	Adds ConsentFn hook and attaches declined-set onto clients before inspection.
internal/models/client.go	Introduces ServerRef and ClientToInspect.Declined runtime state.
internal/inspect/inspector.go	Skips inspection (and thus subprocess startup) for declined stdio servers.
internal/inspect/inspector_test.go	Tests declined-stdio skipping and remote servers unaffected by declines.
internal/consent/consent.go	Implements consent UI, deterministic enumeration, shell quoting, and env redaction.
internal/consent/consent_test.go	Tests consent allow/decline flow, EOF behavior, quoting, and env redaction.
internal/cli/scan.go	Wires consent/CI validation + CI exit evaluation into scan.
internal/cli/inspect.go	Wires consent/CI validation + CI exit evaluation into inspect.
internal/cli/flags.go	Adds common flags for dangerous-run, CI, and ignore issue codes.
internal/cli/consent.go	Implements flag validation, push-key automation detection, consent hook builder, CI exit logic.
internal/cli/consent_test.go	Tests flag validation, push-key detection, consent hook branching, and CI exit filtering.

Comments suppressed due to low confidence (1)

internal/consent/consent.go:95

• When there are no stdio servers (remote-only scan), the summary prints "Proceeding with 0 of 0 stdio servers." which is confusing/no-op. Consider only printing this summary when len(stdioItems) > 0, or adjusting the summary to mention remote servers instead.

	allowed := len(stdioItems) - len(declined)
	msg := fmt.Sprintf("\nProceeding with %d of %d stdio servers.", allowed, len(stdioItems))
	if len(declined) > 0 {
		msg += fmt.Sprintf(" Skipped: %d.", len(declined))
	}
	fmt.Fprintln(out, msg)

---

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

[Copilot]

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.

[Copilot]

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

internal/cli/scan.go:47

• The 5-minute context timeout starts before interactive consent prompts run (ConsentFn is invoked inside the pipeline). If a user takes time to review commands/answer prompts, the context may expire and later discovery/inspection will be canceled unexpectedly. Consider starting the timeout only after consent collection (or using a longer/no timeout for interactive runs, while keeping a timeout for CI/non-interactive runs).

	ctx, cancel := context.WithTimeout(cmd.Context(), 5*time.Minute)
	defer cancel()

[Copilot]

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

internal/consent/consent.go:141

• enumerate appends servers without de-duplicating by models.ServerRef. If the same config file is scanned twice (e.g., duplicate CLI paths), the user may be prompted multiple times for the same stdio server and the final decision becomes order-dependent. Consider de-duping stdio/remote items by (configPath, name) before prompting (while preserving deterministic ordering).

	for _, client := range clients {
		// Deterministic ordering: sort config paths, then server names.
		paths := make([]string, 0, len(client.MCPConfigs))
		for path := range client.MCPConfigs {
			paths = append(paths, path)
		}
		sort.Strings(paths)
		for _, configPath := range paths {
			configOrErr := client.MCPConfigs[configPath]
			if configOrErr.Error != nil || configOrErr.Config == nil {
				continue
			}
			servers := configOrErr.Config.GetServers()
			names := make([]string, 0, len(servers))
			for name := range servers {
				names = append(names, name)
			}
			sort.Strings(names)
			for _, name := range names {
				switch s := servers[name].(type) {
				case *models.StdioServer:
					stdioItems = append(stdioItems, stdioItem{configPath, name, s})
				case *models.RemoteServer:
					remoteItems = append(remoteItems, remoteItem{configPath, name, s})
				}

[Copilot]

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.

[Copilot]

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.

[Copilot]

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.