fix(readers): read n8n include/excludeTools and host MCP keys/files

[pengfei-threemoonslab]

Summary

• Closes readers: read n8n include/excludeTools and the MCP keys and files hosts export #1025
• n8n MCP Client Tool (inputs/n8n/_tools.py). The reader now reads n8n's own include (all / selected / except), with absent meaning all (n8n leaves the default out of an export), plus includeTools and excludeTools. The default export, include: all, include: except, and include: selected with no includeTools each produce one <node>.* wildcard action, a mcp_client_wildcard dynamic surface and SHIP-N8N-MCP-CLIENT-TOOLSET-UNFILTERED. An except node's exclusions go on the artifact record and into the finding's excluded_tools. The legacy spellings stay as aliases, read in a fixed order with include first. Before this, the key was picked from a set, so the result depended on the hash seed.
• Secret forwarding (inputs/mcp_manifest.py). _env_secret_names now reads the whole server declaration. It still reads env, and adds Codex env_vars, bearer_token_env_var (always counted), env_http_headers, and ${NAME} references in headers. These now fire SHIP-MCP-ENV-SECRET-PASSTHROUGH and raise the risk score the same way an env entry does. A value written where a variable name belongs passes through redact_text, so it is never echoed into evidence.
• Coverage overclaim (inputs/codex_config.py, inputs/mcp_manifest.py). The read set and the other hosts' MCP files are now two module constants. The walk matches files from them, and the codex_config coverage text is generated from them, so the two cannot disagree. A .cursor/mcp.json or .vscode/mcp.json under the source root now gets a named source warning (core/source_warnings.unread_host_mcp_config_warning) instead of being skipped silently. docs/determinism-boundary.{md,json} are regenerated.
• Docs: docs/checks.md prose, the fires_when text and evidence_fields in docs/checks/{n8n,mcp_permissions}.yaml, and the regenerated docs/checks.json. CHANGELOG has three bullets under ## Unreleased. STABILITY.md has a migration note (#mcp-reader-conformance-1025) recording which check conditions broaden and which fingerprints move, as § Check IDs requires.

Type

• Check or risk-model change
• Input adapter change
• CLI or GitHub Action behavior
• Report, schema, or SARIF output
• Documentation only

Authorship

• A coding agent wrote some or all of this change. Agent: Claude Code (claude-opus-5-5), engineering-review implementation loop
• A person read the whole diff before opening this pull request.

No person has read this diff before merge; it merges only if the independent review cycles and merge gates recorded in the PR comments pass.

Verification

CI is authoritative for python -m ruff check ., python -m compileall -q src tests, and python -m pytest.

Additional local checks run (Python 3.14, PYTHONPATH=src:.):

• python -m ruff check .: All checks passed.
• python -m compileall -q src tests: ok.
• python scripts/generate_schemas.py --check: exit 0. It regenerated docs/determinism-boundary.{md,json} and docs/checks.json, and those changes are committed.
• python scripts/regenerate_goldens.py --check: Checked 24 sample artifacts; 0 changed. No sample verdict moves.
• python scripts/verify_dependency_lock.py: OK.
• python scripts/build-llms-full.py: regenerated llms-full.txt, because docs/checks.md is one of its sources.
• Regression tests fail before and pass after. I ran them against origin/main's src/ from a scratch copy. 15 of the 17 new cases failed there: the four n8n "every tool" exports, the include precedence case, the four per-key passthrough cases, the two credential-header cases, the bearer redaction case, the two coverage parity tests, and the reported workspace (which was passed on main). The 2 that passed pin behaviour this change keeps: include: selected naming its tools, and non-secret names that are not counted.
• Targeted pytest over the touched area, 47 files covering n8n, mcp_manifest, mcp_permissions, mcp_audit, determinism_boundary, adopter_vocabulary, public_surface_contract, codex_boundary_check, preflight, verify_orchestrator, distribution_surface_parity, schema_roundtrip, metadata and others: all passed except tests/test_codex_boundary_check.py::test_codex_check_boundary_json_golden_outputs, a known pre-existing local-only failure on 3.14.
• ./shipgate verify --workspace . --config shipgate.yaml --base $(git merge-base origin/main HEAD) --head HEAD --json: control.state is complete, every permission is true, release_decision.decision is passed.
• Full suite (pytest -m "not perf and not slow", run serially): started in the background on this head. It takes about 3 hours serially here, so CI's six shards are authoritative, and the local result will be added as a comment if it finishes in this session.

Release-readiness notes

• No user-code import added to default scan paths
• No network access added to default scan paths
• New or changed check IDs are documented in docs/checks.md (no IDs added or renamed; the changed firing conditions of two are documented)
• Report/schema changes are additive or documented in STABILITY.md

Decisions taken

• Narrow and warn, rather than read other hosts' files. The issue says narrowing is the minimum and reading belongs to Read the allowlist files hosts shipped in 2026: GitHub managed-settings MCP allowlist, .github/agents/, Claude managed-settings, Cursor permissions.json, Copilot / Gemini CLI / Windsurf MCP configs #663. So .cursor/mcp.json and .vscode/mcp.json are named, not parsed. They are exactly the host MCP config paths in core/boundary_registry.BOUNDARY_ADAPTERS that this reader does not read, and a parity test fails if the registry gains one that is in neither tuple. There is one warning per file, so each file is a named evidence gap, and the decision is at least review_required.
• Not reusing host_grants._mcp_env_var_names as is. That function is a publication filter. It drops a name that its digest redaction rewrites, for example API_KEY after TOKEN in one env_vars list publishes only TOKEN. For detection that would fail open. The reader covers the same key set as Show concrete field changes so an unfamiliar reviewer can act on the first advisory result #795 and uses detection semantics instead: a name counts when it is env-var-shaped and either secret-like or carried by a credential-named header. redact_text runs on each name before it is published.
• bearer_token_env_var always counts. It holds a bearer token whatever the variable is called.
• headers is read only for ${NAME} references, and Codex's http_headers is not read. Codex documents http_headers as static values, so a reference there is not expanded. A literal secret in it is a hardcoded secret, not forwarding, and is out of scope.
• include: selected with no includeTools is read as all. Upstream getSelectedTools returns every tool in that case (if (!includeTools?.length) return tools;).
• A node with no include but a legacy tool list (tools, toolName, …) is still read as selected. This keeps the aliases working, for example for the community n8n-nodes-mcp node. A real first-party export cannot carry includeTools without include: selected, because n8n drops hidden parameters on export.
• excluded_tools is added to the finding evidence only when non-empty. An existing all_except finding that excludes nothing keeps its fingerprint.
• The MCP Server Trigger is unchanged. _selection_mode(..., client=True) is the only path that reads include, treats except as all-except, reads a missing selection as all, and reads an empty selected as all. The trigger reads only the earlier spellings, as on main (review cycle 1, C1-1).
• Env references. ${env:NAME} is read as NAME, ${NAME:-default} is still read as NAME, and any other ${prefix:…} (VS Code's ${input:id}) names no variable (review cycle 1, C1-2).

Protected paths touched

none

Follow-up candidates

• src/agents_shipgate/inputs/n8n/_common.py:87: _top_level_string(value, keys: set[str]) takes the first present key in set iteration order, which follows the per-process hash seed. When a node carries more than one of the keys, the value it picks can differ from run to run. Affected callers include {"url", "path", "endpoint"} in _auth_risk.py:168 and _common.py:284, the workflow-target keys in _tools.py:323, and {"authentication", "authType", "authorization"} in _workflows.py:435. This PR removes that dependence only from the MCP selection mode.
• Reading .cursor/mcp.json and .vscode/mcp.json in codex_config, instead of naming them, is Read the allowlist files hosts shipped in 2026: GitHub managed-settings MCP allowlist, .github/agents/, Claude managed-settings, Cursor permissions.json, Copilot / Gemini CLI / Windsurf MCP configs #663.

[pengfei-threemoonslab]

🤖 Engineering review — independent review, cycle 1
Reviewed head: dd67128

No P0–P2 findings.

Two P3 notes, neither blocking:

Severity	File:line	What is wrong	Why	Suggested fix
P3	src/agents_shipgate/inputs/n8n/_tools.py:491	The MCP Server Trigger path (_is_unfiltered_mode, called from _workflows.py:319) shares _selection_mode, so it now reads include and treats selected with no tool list as all. On origin/main, _is_unfiltered_mode returns False for {"include": "all"}, {"include": "except"} and {"toolSelection": "selected"}. On this head it returns True for all three, and each adds an mcp_server_wildcard dynamic surface.	The PR body says "The MCP Server Trigger is unchanged". It is unchanged for the real n8n node, which has none of these parameters, but parameter shapes that are not n8n's own now behave differently, and CHANGELOG/STABILITY do not say so.	Either keep the old trigger semantics (pass a flag so only the client node takes include and the empty-selected → all rule), or add one clause to the STABILITY note.
P3	src/agents_shipgate/inputs/mcp_manifest.py:1075	A credential header whose value uses VS Code/Cursor reference syntax, such as .mcp.json "headers": {"Authorization": "Bearer ${env:GITHUB_TOKEN}"}, publishes env_secret_names: ["env"]. _ENV_REF_RE stops at :, and the credential-carrier rule counts whatever it captured. ${input:token} gives "input" in the same way.	The finding still fires, which is the safe direction, but the evidence names a variable that does not exist. Claude Code does not expand ${env:…}, so this only happens in files written for other hosts.	Skip a capture followed by :, or read ${env:NAME} as NAME.

Points checked

1. n8n include / includeTools / excludeTools. I built @n8n/n8n-nodes-langchain.mcpClientTool nodes (typeVersion 1.1 endpointUrl, and a v1 sseEndpoint) wired to an agent over ai_tool, and ran run_scan (production target) on this head and on origin/main:

   Parameters	origin/main	this head
   default export {endpointUrl, options:{}}	GitHub MCP, dyn 0, no UNFILTERED	GitHub MCP.*, dyn 1, UNFILTERED high all
   v1 {sseEndpoint}	same as above	same as above
   include: all	concrete, no finding	wildcard, UNFILTERED all
   include: selected + includeTools	get_issue, list_issues	same, dyn 0, no UNFILTERED
   include: selected, no / empty includeTools	concrete	wildcard, UNFILTERED all (as upstream if (!includeTools?.length) return tools)
   include: except + excludeTools	concrete	wildcard, UNFILTERED all_except, excluded_tools: ["list_issues"]
   include: except, no / empty excludeTools	concrete	UNFILTERED all_except, no excluded_tools
   include: all + stale includeTools	x (wrong)	wildcard (hidden param ignored)
   include as expression	concrete	wildcard + mcp_client_selection_mode_unknown, no UNFILTERED (existing unknown-mode design)
   excludeTools: ["ghp_…"]	n/a	["[REDACTED:github_token]"]

   Each row matches upstream getSelectedTools. On these inputs the decision moves from insufficient_evidence to review_required, the higher-precedence "named high concern" branch.

2. Codex secret forwarding. I scanned one workspace containing env_vars, bearer_token_env_var (a name, and a ghp_… value), env_http_headers (Authorization → name, plus a non-credential header carrying a secret-like name, plus one carrying a Bearer sk-… value), static http_headers with an sk-… literal, and .mcp.json headers with ${NAME}, $(NAME) and a literal sk-…. Each forwarding form fires SHIP-MCP-ENV-SECRET-PASSTHROUGH with the planted name, and the bearer value is published as [REDACTED:github_token]. I grepped every produced file (report.json, report.md, report.sarif, packet.*, current-control.json) for the four planted values: none appears. Only the variable names do.

3. Unread other-host files. These are CLI runs, this head against origin/main, with --ci-mode advisory and --ci-mode strict:

   • The reported workspace (reviewed .codex/config.toml plus .cursor/mcp.json and .vscode/mcp.json): review_required in both modes on this head, passed on main. Each file gets its own named warning, which becomes an evidence_gaps[] source_warning.
   • A nested pkg/sub/.cursor/mcp.json: review_required in both modes (main: passed).
   • A workspace with only .cursor/mcp.json or only .vscode/mcp.json: insufficient_evidence with the named warning (main: the same decision, no warning).
   • The same workspace with only .codex/config.toml: still passed.
   • Two overlapping codex_config roots: the warning is reported once.

   So the warning holds the decision at review_required or above in both modes.

4. Fingerprints. regenerate_goldens.py --check reports 24 sample artifacts with 0 changed. No sample, example or fixture contains an MCP client node in a non-selected mode, a Codex forwarding key, or a .cursor/.vscode mcp.json. The only tracked MCP client node (benchmark/safety-qualification/constructed/n8n_agent_adds_crm_mcp_client) is include: selected with includeTools, so its output is unchanged. excluded_tools is added to evidence only when it is non-empty, so a legacy all_except finding that excludes nothing keeps its fingerprint. That matches the STABILITY note.

Contract and hygiene:

• The CHANGELOG has three accurate bullets, and the STABILITY migration note #mcp-reader-conformance-1025 is present.
• No check ID is added or renamed. excluded_tools is an additive evidence field.
• docs/determinism-boundary.{md,json} and docs/checks.json are regenerated, and llms-full.txt is rebuilt.
• prompts/, skills/ and .github/ are untouched.
• No commit trailers, and no added text addressing an AI or reviewer.
• The declined reuse of host_grants._mcp_env_var_names is justified: it is a publication filter, and reusing it for detection would fail open.

Commands run

• git diff origin/main...dd671286; issue readers: read n8n include/excludeTools and the MCP keys and files hosts export #1025 and the private repro details; PR body and comments
• pytest tests/test_n8n.py tests/test_mcp_manifest.py tests/test_adopter_vocabulary.py tests/test_determinism_boundary.py tests/test_mcp_permissions.py tests/test_distribution_surface_parity.py: pass
• pytest tests/test_safety_qualification.py tests/test_beta_strata_inventory.py tests/test_mcp_env_var_names.py tests/test_mcp_permissions.py: pass
• pytest tests/test_mcp_*.py tests/test_host_boundary_unread_surfaces.py tests/test_host_grant_direction.py tests/test_vscode_mcp_*.py tests/test_surface_exclusions.py tests/test_public_surface_contract.py tests/test_codex_boundary_check.py tests/test_schema_roundtrip.py: one failure, test_codex_boundary_check.py::test_codex_check_boundary_json_golden_outputs. This is the known pre-existing local-only failure on Python 3.14 (the TOML parse-error text differs). Everything else passed.
• Full suite: not run locally. The change is confined to three readers, so CI's shards are authoritative.
• ruff check .: All checks passed. compileall -q src tests: ok. generate_schemas.py --check: exit 0, tree clean. regenerate_goldens.py --check: 24 checked, 0 changed. verify_dependency_lock.py: OK
• n8n probe (12 export shapes) and secret/host-file probes, run against this head and an origin/main snapshot, as described above

[pengfei-threemoonslab]

🤖 Engineering review — implementer, addressed cycle 1

Head: 227a21d

Finding	Disposition
C1-1 (P3): the MCP Server Trigger path (_is_unfiltered_mode) picked up the client node's include reading and its empty-selected→all rule	Fixed in 227a21d. I took the flag option: it keeps the trigger's old semantics, so the PR body's "trigger unchanged" statement holds and no STABILITY clause is needed. _selection_mode(parameters, client=True) is now the only path that reads include, accepts except, reads a missing selection as all, and reads an empty selected as all. The trigger reads only toolSelection/toolsToInclude/toolSelectionMode, as on main, in a fixed order. New test test_n8n_mcp_server_trigger_keeps_the_selection_it_read pins both nodes: {include: all}, {include: except}, {toolSelection: selected} and {} are not unfiltered on the trigger, and the legacy All/All Except still are. The same cases pin the client mode for each shape.
C1-2 (P3): _ENV_REF_RE stopped at :, so ${env:GITHUB_TOKEN} published env and ${input:x} published input	Fixed in 227a21d. The pattern now reads ${env:NAME} as NAME and keeps ${NAME:-default} as NAME. A capture followed by any other : is skipped, so ${input:id} names no variable. The fix applies to env values as well as headers. New test test_a_prefixed_reference_names_the_variable_not_its_prefix covers ${env:…}, ${…:-} and ${input:…} in a credential header. CHANGELOG, docs/checks.md and llms-full.txt are updated.

Checks on 227a21d:

• ruff, compileall, generate_schemas --check and verify_dependency_lock: pass.
• regenerate_goldens --check: 0 of 24 changed.
• Tests: test_n8n, test_mcp_manifest, test_mcp_audit, test_mcp_permissions, test_adopter_vocabulary, test_determinism_boundary, test_p0_binding_canaries, test_inputs and test_llms_full_is_up_to_date all pass.
• ./shipgate verify on the branch: complete, all permissions true, decision passed.

[pengfei-threemoonslab]

🤖 Engineering review — independent review, cycle 2
Reviewed head: 227a21d

Narrow delta review of git diff dd671286 227a21d3 (the two cycle-1 P3 fixes).

No P0–P2 findings.

Severity	File:line	What is wrong	Why	Suggested fix
P3	src/agents_shipgate/inputs/mcp_manifest.py:48	${NAME:?message} and ${NAME:=default} now name no variable (on origin/main and dd67128 they read NAME)	The lookahead (?!:(?!-)) rejects every : except :-, so the other POSIX operator forms drop out along with ${input:…}. Repro: _env_secret_names({"headers": {"Authorization": "${GITHUB_TOKEN:?missing}"}}) gives []; {"env": {"FOO": "${GITHUB_TOKEN:?x}"}} also gives [], where main counted FOO. These forms are uncommon in host MCP configs, so this does not block.	Reject only a : followed by an identifier character, e.g. (?!:[A-Za-z_]), so that :-, :?, := and :+ still read NAME while ${input:id} / ${config:x} do not; add ${NAME:?x} to the parametrized test.

Verification

• Trigger path vs origin/main: differential run over 320 parameter shapes (each of include/toolSelection/toolsToInclude/toolSelectionMode × 10 mode values × 4 includeTools shapes × with/without excludeTools) comparing _is_unfiltered_mode with origin/main's _selection_mode: 0 differences. The listed shapes: the default {endpointUrl}, include: all, include: except + excludeTools, and include: selected are all not unfiltered on the trigger, as on main; toolSelection: all / toolsToInclude: All stay unfiltered. Legacy-key precedence is now ordered rather than set-order, a deterministic improvement.
• Client path vs upstream getSelectedTools: {endpointUrl} → all; include: all → all; include: except + excludeTools → all_except; include: selected + includeTools → selected; include: selected with no tools → all (upstream returns every tool when includeTools is empty). Matches.
• _ENV_REF_RE / _env_secret_names: Bearer ${env:GITHUB_TOKEN} → GITHUB_TOKEN; ${env:API_KEY} in X-Api-Key → API_KEY; ${GITHUB_TOKEN:-none} → GITHUB_TOKEN; ${input:github_token} → none (header and env); env FOO: ${env:GITHUB_TOKEN} / ${GITHUB_TOKEN:-x} → FOO; ${env:A}-${env:B} → A, B; $env:X, ${config:x}, ${env:} → none.
• pytest tests/test_mcp_manifest.py tests/test_n8n.py: 96 passed
• ruff check .: All checks passed
• compileall -q src tests: ok
• scripts/generate_schemas.py --check: rc 0
• scripts/regenerate_goldens.py --check: 24 checked, 0 changed
• scripts/verify_dependency_lock.py: OK
• Commits dd67128..227a21d: no attribution trailers. The CHANGELOG, docs/checks.md and llms-full.txt wording matches the behaviour.
• Full suite: not run (narrow delta).

Earlier findings

ID	Disposition
C1-1 (trigger unfiltered-mode semantics changed)	resolved. The trigger reads only the legacy spellings, with no differences from origin/main across 320 shapes; test_n8n_mcp_server_trigger_keeps_the_selection_it_read pins it
C1-2 (${env:NAME} publishes env)	resolved. ${env:NAME} reads as NAME in headers and env values, and ${input:…} names nothing; see the new P3 above for the :?/:= side effect

[pengfei-threemoonslab]

🤖 Engineering review — implementer, addressed cycle 2

Head: 53d9ac8

Finding

Disposition

C2 (P3): the cycle-1 lookahead (?!:(?!-)) rejected every : except :-, so ${NAME:?msg} and ${NAME:=default} stopped naming NAME

Fixed in 53d9ac8. The lookahead is now (?!:[A-Za-z_]), which skips a : only when an identifier character follows it, as in ${prefix:id}. ${NAME:-x}, ${NAME:?x} and ${NAME:=x} name NAME again, as on main. ${env:NAME} still reads NAME, and ${input:id} still names nothing. Your repro, _env_secret_names({'env': {'FOO': '${GITHUB_TOKEN:?x}'}}), gives ['FOO'], the same as origin/main. I added ${GITHUB_TOKEN:?x} and ${GITHUB_TOKEN:=d} to test_a_prefixed_reference_names_the_variable_not_its_prefix. The docs/checks.md line still holds, so I left it unchanged; the code comment above the regex now lists the shell forms. No other edits.

Checks on 53d9ac8:

• Static checks: ruff, compileall, generate_schemas --check and verify_dependency_lock pass, and regenerate_goldens --check reports 0 of 24 changed.
• Tests: test_mcp_manifest, test_mcp_audit and test_mcp_permissions pass.
• ./shipgate verify returns complete, with all permissions true and decision passed.

[pengfei-threemoonslab]

🤖 Engineering review — merge readiness

• Reviewed head: 227a21d32730c0a5b8b2266fae4cffa285912f4d (cycle 2, clean on P0–P2). Current head 53d9ac8ca42041ff89c8ac60bf19686f732d33a7 differs only by the _ENV_REF_RE lookahead, its comment and two parametrized test cases that resolve cycle 2's P3 (${NAME:?x} / ${NAME:=d} regression), checked by the orchestrator.
• Cycle 1 confirmed 12 n8n MCP Client Tool shapes against upstream getSelectedTools, every Codex forwarding key, no planted secret in any output, and the review_required hold for unread .cursor/mcp.json / .vscode/mcp.json; cycle 2 ran a 320-shape differential of the trigger path against origin/main with 0 differences. Findings C1-1 (trigger semantics drift), C1-2 and C2-1 (env-reference naming) all resolved.
• Follow-up filed: n8n/MCP readers: deterministic key choice in _top_level_string, a specific next_action for unread other-host MCP files, exact mcpClientTool matching #1073 (deterministic key choice in _top_level_string, a specific next_action for unread other-host MCP files, exact mcpClientTool matching). Reading the other hosts' files remains Read the allowlist files hosts shipped in 2026: GitHub managed-settings MCP allowlist, .github/agents/, Claude managed-settings, Cursor permissions.json, Copilot / Gemini CLI / Windsurf MCP configs #663.
• CI on 53d9ac8: all required checks succeeded; release-tag-consistency skipped as expected.
• Repository gate (./shipgate verify, orchestrator run): state complete, merge permission true, decision passed.
• Branch contains origin/main (including fix(verify): fail closed without a base or with a deleted manifest #1070); no attribution trailers; no protected paths; no unanswered human comments.