nexus-mcp/.github/prompts/plan-fixPsCommandInjectionAdAdapter.prompt.md

Plan: Fix PowerShell Command Injection in ad_adapter.py

🌿 Branch Identity

Suggested Branch Name: fix/ps-command-injection-ad-adapter

Purpose: Closes CRITICAL #2 from the code health report — eliminates a PowerShell command injection vulnerability (OWASP A03) in nexus-mcp/lib/ad_adapter.py before any production connection to the AD server.

---

🚀 The Fix: Parameterized PowerShell Execution

Plain English: Right now, when a username like jane.doe is looked up, Python builds a PowerShell command as a string with the username baked into it — like handing someone a note that says "search for jane.doe". A malicious input like '; Remove-Item C:\Important; ' would make the note say "search for nothing, then delete files."

The fix passes user values through a separate channel — OS environment variables — so they can never change the structure of the command, only its data. PowerShell reads them via $env:NEXUS_ARG0, exactly like reading from a locked box, not from the note itself. No new libraries are needed.

---

💻 Implementation Plan

This is a 5-phase change, all contained to one file plus its test file.

Phase 1 — Harden _run_powershell() (blocks all other phases)

In nexus-mcp/lib/ad_adapter.py:

1. Add import os to the top-of-file imports (line 1–6 block).
2. Change the _run_powershell() signature to add env_params: dict[str, str] | None = None.
3. Before the create_subprocess_exec call, build: proc_env = {**os.environ, **(env_params or {})}.
4. Pass env=proc_env to asyncio.create_subprocess_exec(...).
5. Fix the cred_block injection surface: replace '{escaped_password}' and '{self.username}' with $env:NEXUS_PS_PASSWORD and $env:NEXUS_PS_USERNAME in the PowerShell string. Inject those values via proc_env from self.password / self.username directly (they are server-config values, not user input, but this eliminates the escape-quote surface entirely).

Phase 2 — Fix get_user(), get_user_groups(), get_group_members(), get_computer() (parallel — all follow the same pattern)

For each:

1. Add a length guard at the top: if len(input_var) > 256: raise ValueError(...).
2. Remove the _escape_ps_single_quoted() call.
3. Pass the value via env_params={"NEXUS_ARG0": input_var} in the _run_powershell() call.
4. In the PowerShell command string, replace '{escaped_X}' with '$($env:NEXUS_ARG0)'.

Before (line ~113 in get_user()):

escaped_username = self._escape_ps_single_quoted(username)
command = f"""
    $user = Get-ADUser -Filter "samAccountName -eq '{escaped_username}'" ...
"""
result = await self._run_powershell(command, {"username": username})

After:

if len(username) > 256:
    raise ValueError("username too long")
command = """
    $user = Get-ADUser -Filter "samAccountName -eq '$($env:NEXUS_ARG0)'" ...
"""
result = await self._run_powershell(command, {"username": username}, env_params={"NEXUS_ARG0": username})

Phase 3 — Fix search_users_by_name() (parallel with Phase 2)

1. Add length guard: if len(query.strip()) > 100: raise ValueError(...).
2. Remove escaped_query interpolation.
3. Pass env_params={"NEXUS_ARG0": query}.
4. In the PowerShell script, add $query = $env:NEXUS_ARG0 as the first line. All downstream PS-internal variable uses ($query, $tokens, $adFilter) already operate on that PS variable — they stay as-is.

Phase 4 — Fix query_users(), count_users(), summarize_users() (parallel with Phase 2)

These build dynamic filters from multiple string params. Each filter string value gets its own env var:

Python key	Env var	Max length
name_contains	NEXUS_FILTER_NAME	256
username_prefix	NEXUS_FILTER_PREFIX	64
ou_contains	NEXUS_FILTER_OU	512
description_contains	NEXUS_FILTER_DESC	256
group_any (JSON)	NEXUS_FILTER_GROUPS	2048

In the PowerShell filter fragment, replace the Python-interpolated value with the env var reference. For example:

# Before (Python builds this):  "DisplayName -like '*{escaped_query}*'"
# After: "DisplayName -like '*$($env:NEXUS_FILTER_NAME)*'"

Collect all env vars into a single env_params dict and pass once to _run_powershell().

Phase 5 — Tests (depends on Phases 1–4)

Add to nexus-mcp/tests/identity_tests/test_ad_adapter.py:

1. test_get_user_injection_attempt — calls get_user("'; Remove-Item C:\\test; '") with _run_powershell mocked. Asserts it returns None (not raised, not crashed) — proving the method still works with hostile input.
2. test_search_users_injection_attempt — same for search_users_by_name.
3. test_run_powershell_passes_env_params — mocks asyncio.create_subprocess_exec, calls _run_powershell("echo test", env_params={"NEXUS_ARG0": "val"}), asserts the env kwarg passed to create_subprocess_exec contains "NEXUS_ARG0": "val".
4. test_get_user_input_too_long — asserts ValueError is raised for a 257-char username.

Note on existing test impact: The cred_block fix in Phase 1 affects test_run_powershell_with_credentials — that test currently asserts on the command string content. It will need updating to assert on env var presence instead of password interpolation in the command string.

---

🛠️ Deployment Steps

# 1. Create and switch to the feature branch
git checkout -b fix/ps-command-injection-ad-adapter

# 2. No new packages required — os is stdlib, pyproject.toml is unchanged

# 3. Run existing tests before making changes (baseline)
cd nexus-mcp
python -m pytest tests/identity_tests/test_ad_adapter.py -v

# 4. Implement the phases above in ad_adapter.py and test_ad_adapter.py

# 5. Run tests after changes
python -m pytest tests/identity_tests/test_ad_adapter.py -v

# 6. Run full nexus-mcp test suite to confirm no regressions
python -m pytest tests/ -v --ignore=tests/integration_test_audit_shard.py

---

🧪 How to Verify It Works

Success metric — automated:

tests/identity_tests/test_ad_adapter.py::TestActiveDirectoryBackend::test_get_user_injection_attempt PASSED
tests/identity_tests/test_ad_adapter.py::TestActiveDirectoryBackend::test_run_powershell_passes_env_params PASSED
tests/identity_tests/test_ad_adapter.py::TestActiveDirectoryBackend::test_get_user_input_too_long PASSED

Success metric — code audit (zero output expected):

# Should return nothing — no more _escape_ps_single_quoted calls in method bodies
grep -n "_escape_ps_single_quoted(" nexus-mcp/lib/ad_adapter.py | grep -v "def _escape"

# Should return nothing — no more Python f-string interpolation of user values into PS command strings
grep -n "escaped_" nexus-mcp/lib/ad_adapter.py

Out of scope: archive/Identity/ tree. The static method _escape_ps_single_quoted() remains in place, marked # DEPRECATED, to avoid breaking archive callers.